From: Vsevolod Stakhov Date: Thu, 23 Jul 2026 09:32:23 +0000 (+0100) Subject: [Fix] content_type: bound parameters per header X-Git-Tag: 4.1.3~43 X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=4ecae17a7e34e76547def02108cb3bed2db1d680;p=thirdparty%2Frspamd.git [Fix] content_type: bound parameters per header A single Content-Type or Content-Disposition field can carry an unbounded number of parameters. Each one materialises a separate pool object plus a hash/list entry, so a message bounded at max_message (50 MiB by default) could amplify into millions of allocations from one folded header (~20x for Content-Type, more for Content-Disposition which copies name and value per param). The existing header-count cap limits header *fields*, not parameters within a field, so it does not help here. Cap the number of parameters materialised per header at 1024 (real messages use a handful). Content-Type flags the truncation as BROKEN; Content-Disposition has no flags field and simply stops. The counter counts parameters, not distinct names, so a same-name flood is bounded too. Also fix rspamd_cmp_pieces: it subtracted two unsigned RFC 2231 continuation ids and truncated to int32_t, which overflows when the ids differ by more than INT32_MAX and reverses the reconstruction order. Use an explicit comparison instead. Adds a C++ unit suite covering the parameter caps and the ordering fix. --- diff --git a/src/libmime/content_type.c b/src/libmime/content_type.c index 07c61b6711..6e1820b2a1 100644 --- a/src/libmime/content_type.c +++ b/src/libmime/content_type.c @@ -20,6 +20,14 @@ #include "libserver/url.h" #include "libmime/mime_encoding.h" +/* + * Upper bound on the number of parameters we materialise for a single + * Content-Type or Content-Disposition header. Real messages use a handful; + * this cap exists purely to stop a crafted header (e.g. `text/plain; a=;a=;...`) + * from amplifying a bounded input into millions of per-parameter allocations. + */ +static const unsigned int max_content_type_params = 1024; + static gboolean rspamd_rfc2231_decode(rspamd_mempool_t *pool, struct rspamd_content_type_param *param, @@ -166,7 +174,19 @@ rspamd_param_maybe_rfc2231_process(rspamd_mempool_t *pool, static int32_t rspamd_cmp_pieces(struct rspamd_content_type_param *p1, struct rspamd_content_type_param *p2) { - return p1->rfc2231_id - p2->rfc2231_id; + /* + * Explicit comparison: subtracting two unsigned ids and truncating to + * int32_t overflows when they differ by more than INT32_MAX, which yields + * an inconsistent ordering for the sort below. + */ + if (p1->rfc2231_id < p2->rfc2231_id) { + return -1; + } + else if (p1->rfc2231_id > p2->rfc2231_id) { + return 1; + } + + return 0; } static void @@ -317,6 +337,13 @@ void rspamd_content_type_add_param(rspamd_mempool_t *pool, g_assert(ct != NULL); + if (ct->nparams >= max_content_type_params) { + /* Refuse to amplify a crafted header into unbounded allocations */ + ct->flags |= RSPAMD_CONTENT_TYPE_BROKEN; + return; + } + + ct->nparams++; nparam = rspamd_mempool_alloc0(pool, sizeof(*nparam)); rspamd_str_lc(name_start, name_end - name_start); @@ -810,6 +837,12 @@ void rspamd_content_disposition_add_param(rspamd_mempool_t *pool, g_assert(cd != NULL); + if (cd->nparams >= max_content_type_params) { + /* Refuse to amplify a crafted header into unbounded allocations */ + return; + } + + cd->nparams++; name_cpy = rspamd_mempool_alloc(pool, name_end - name_start); memcpy(name_cpy, name_start, name_end - name_start); name_cpy_end = name_cpy + (name_end - name_start); diff --git a/src/libmime/content_type.h b/src/libmime/content_type.h index 2177cdf2dd..cb19a6fa1c 100644 --- a/src/libmime/content_type.h +++ b/src/libmime/content_type.h @@ -59,7 +59,8 @@ struct rspamd_content_type { rspamd_ftok_t boundary; rspamd_ftok_t orig_boundary; enum rspamd_content_type_flags flags; - GHashTable *attrs; /* Can be empty */ + unsigned int nparams; /* Number of parameters added (saturates at the cap) */ + GHashTable *attrs; /* Can be empty */ }; enum rspamd_content_disposition_type { @@ -72,7 +73,8 @@ struct rspamd_content_disposition { char *lc_data; enum rspamd_content_disposition_type type; rspamd_ftok_t filename; - GHashTable *attrs; /* Can be empty */ + unsigned int nparams; /* Number of parameters added (saturates at the cap) */ + GHashTable *attrs; /* Can be empty */ }; /** diff --git a/test/rspamd_cxx_unit.cxx b/test/rspamd_cxx_unit.cxx index ee9a71d089..3a2329f79b 100644 --- a/test/rspamd_cxx_unit.cxx +++ b/test/rspamd_cxx_unit.cxx @@ -37,6 +37,7 @@ #include "rspamd_cxx_unit_upstream_latency.hxx" #include "rspamd_cxx_unit_upstream_srv.hxx" #include "rspamd_cxx_unit_multipart.hxx" +#include "rspamd_cxx_unit_content_type.hxx" #include "rspamd_cxx_unit_content_negotiation.hxx" #include "rspamd_cxx_unit_settings_merge.hxx" #include "rspamd_cxx_unit_fpconv.hxx" diff --git a/test/rspamd_cxx_unit_content_type.hxx b/test/rspamd_cxx_unit_content_type.hxx new file mode 100644 index 0000000000..ffc25f2901 --- /dev/null +++ b/test/rspamd_cxx_unit_content_type.hxx @@ -0,0 +1,173 @@ +/* + * Copyright 2025 Vsevolod Stakhov + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef RSPAMD_CXX_UNIT_CONTENT_TYPE_HXX +#define RSPAMD_CXX_UNIT_CONTENT_TYPE_HXX + +#define DOCTEST_CONFIG_IMPLEMENTATION_IN_DLL +#include "doctest/doctest.h" + +#include "libmime/content_type.h" + +#include +#include + +/* + * Mirrors the (file-static) max_content_type_params cap in + * src/libmime/content_type.c. Kept in sync deliberately: if the cap changes, + * this test should be updated alongside it. + */ +static constexpr unsigned int expected_param_cap = 1024; + +TEST_SUITE("content_type") +{ + TEST_CASE("normal parameters are not truncated") + { + rspamd_mempool_t *pool = rspamd_mempool_new(rspamd_mempool_suggest_size(), + "ct-test", 0); + std::string hdr = "text/plain; charset=utf-8; boundary=xyz"; + + auto *ct = rspamd_content_type_parse(hdr.data(), hdr.size(), pool); + REQUIRE(ct != nullptr); + + std::string_view type{ct->type.begin, ct->type.len}; + std::string_view subtype{ct->subtype.begin, ct->subtype.len}; + CHECK(type == "text"); + CHECK(subtype == "plain"); + CHECK(ct->nparams == 2); + CHECK((ct->flags & RSPAMD_CONTENT_TYPE_BROKEN) == 0); + + rspamd_mempool_delete(pool); + } + + TEST_CASE("parameter flood is bounded and flagged broken") + { + rspamd_mempool_t *pool = rspamd_mempool_new(rspamd_mempool_suggest_size(), + "ct-test", 0); + /* + * A single Content-Type value packed with far more parameters than any + * real message would carry. Without the cap this materialises one pool + * object + hash entry per parameter, amplifying a bounded input into a + * huge allocation count. + */ + std::string hdr = "text/plain"; + for (int i = 0; i < 5000; i++) { + hdr += "; p" + std::to_string(i) + "=v" + std::to_string(i); + } + + auto *ct = rspamd_content_type_parse(hdr.data(), hdr.size(), pool); + REQUIRE(ct != nullptr); + + /* The declared type must still be parsed correctly */ + std::string_view type{ct->type.begin, ct->type.len}; + std::string_view subtype{ct->subtype.begin, ct->subtype.len}; + CHECK(type == "text"); + CHECK(subtype == "plain"); + + /* Parameter count saturates at the cap and the truncation is signalled */ + CHECK(ct->nparams == expected_param_cap); + CHECK((ct->flags & RSPAMD_CONTENT_TYPE_BROKEN) != 0); + + rspamd_mempool_delete(pool); + } + + TEST_CASE("duplicate-name flood is bounded too") + { + rspamd_mempool_t *pool = rspamd_mempool_new(rspamd_mempool_suggest_size(), + "ct-test", 0); + /* + * Repeating the same parameter name keeps the hash table at a single + * entry but still allocates one param struct per occurrence, so the cap + * must count params, not distinct names. + */ + std::string hdr = "text/plain"; + for (int i = 0; i < 5000; i++) { + hdr += "; a=b"; + } + + auto *ct = rspamd_content_type_parse(hdr.data(), hdr.size(), pool); + REQUIRE(ct != nullptr); + CHECK(ct->nparams == expected_param_cap); + CHECK((ct->flags & RSPAMD_CONTENT_TYPE_BROKEN) != 0); + + rspamd_mempool_delete(pool); + } + + TEST_CASE("content-disposition parameter flood is bounded") + { + rspamd_mempool_t *pool = rspamd_mempool_new(rspamd_mempool_suggest_size(), + "cd-test", 0); + std::string hdr = "attachment"; + for (int i = 0; i < 5000; i++) { + hdr += "; p" + std::to_string(i) + "=v" + std::to_string(i); + } + + auto *cd = rspamd_content_disposition_parse(hdr.data(), hdr.size(), pool); + REQUIRE(cd != nullptr); + CHECK(cd->type == RSPAMD_CT_ATTACHMENT); + CHECK(cd->nparams == expected_param_cap); + + rspamd_mempool_delete(pool); + } + + TEST_CASE("content-disposition normal filename is preserved") + { + rspamd_mempool_t *pool = rspamd_mempool_new(rspamd_mempool_suggest_size(), + "cd-test", 0); + std::string hdr = "attachment; filename=report.pdf"; + + auto *cd = rspamd_content_disposition_parse(hdr.data(), hdr.size(), pool); + REQUIRE(cd != nullptr); + CHECK(cd->type == RSPAMD_CT_ATTACHMENT); + CHECK(cd->nparams == 1); + + std::string_view fname{cd->filename.begin, cd->filename.len}; + CHECK(fname == "report.pdf"); + + rspamd_mempool_delete(pool); + } + + TEST_CASE("rfc2231 piecewise reconstruction orders by id, not by wraparound") + { + rspamd_mempool_t *pool = rspamd_mempool_new(rspamd_mempool_suggest_size(), + "ct-test", 0); + /* + * Two continuation pieces of one parameter whose ids differ by more + * than INT32_MAX. The old comparator subtracted the two unsigned ids + * and truncated to int32_t, which wraps and reverses the order. The + * reconstructed value must be "ab" (id 0 then id 3000000000), not "ba". + */ + std::string hdr = "text/plain; x*0=a; x*3000000000=b"; + + auto *ct = rspamd_content_type_parse(hdr.data(), hdr.size(), pool); + REQUIRE(ct != nullptr); + REQUIRE(ct->attrs != nullptr); + + rspamd_ftok_t key; + key.begin = "x"; + key.len = 1; + auto *param = (struct rspamd_content_type_param *) + g_hash_table_lookup(ct->attrs, &key); + REQUIRE(param != nullptr); + + std::string_view value{param->value.begin, param->value.len}; + CHECK(value == "ab"); + + rspamd_mempool_delete(pool); + } +} + +#endif// RSPAMD_CXX_UNIT_CONTENT_TYPE_HXX