rspamadm never called rspamd_upstreams_library_config(), so the
upstreams context had neither an event loop nor a resolver and stayed
unconfigured. Host names still worked because they are resolved
synchronously while the config is parsed, but the SRV form
`servers = "service=fuzzy+rspamd.com"` creates a placeholder that is
not selectable until asynchronous resolution fills in its members -
which was never scheduled. Every fuzzy storage configured that way
looked empty: `rspamadm fuzzyping -l` printed no servers at all and
pinging failed with "no fuzzy storage upstream available".
Configure the library right after the resolver is created, before any
command loads its own configuration, so that upstreams created later
schedule resolution themselves in rspamd_upstream_set_active().
That alone is not enough for one shot commands: the resolve timer is
armed for the next loop iteration while fuzzy_ping issued its first
ping straight after loading the config, and the loop only runs while
session events are pending. Add lua_fuzzy.wait_for_storages() which
waits for the selected rules to have servers (bounded by the request
timeout) and use it in fuzzy_ping and fuzzy_hash. An explicit
-s/--server override skips the wait since it bypasses the configured
upstreams.
Also report the rule name instead of nil when a ping cannot be started
without a server override.
[CritFix] controller: fail closed on malformed password
rspamd_is_encrypted_password() only validates the `$<id>` prefix of a
configured password, not that a salt and a key follow it. A password
such as `$1$` therefore passed that check, reached
rspamd_check_encrypted_password() with both components absent, skipped
the comparison block entirely and returned the initial `ret = TRUE`.
Any supplied password then authenticated and was cached as the valid
one.
Initialise `ret` to FALSE and set it only after a completed KDF
comparison, and log the malformed hash instead of silently accepting
it.
rspamd_encrypted_password_get_str() also indexed `password + 3` without
checking the length, reading past the terminator for short strings,
which made the fail-open depend on adjacent heap bytes. Pass the length
in and bail out when the component is missing.
Validate the complete encoded hash at startup as well:
rspamd_controller_password_sane() re-ran the same prefix-only test and
only warned when a password was not encrypted, so a malformed one
passed silently. It now decodes the salt and the key and checks them
against the expected lengths of the pbkdf matching the hash id, which
also fixes the hardcoded pbkdf_list[0] and drops a vacuous g_assert().
[Fix] url: consult the Lua URL filter twice, not per byte
Once the user field exceeded max_email_user, the parser consulted the
Lua filter on every subsequent byte, and each call re-scanned the whole
prefix (control characters, UTF-8 validation, @ counting, custom
filters). The 512 byte user limit in Lua never applies here because the
mid-parse fragment does not contain @ yet, so this ran up to the 16Kb
threshold: quadratic scanning plus thousands of C -> Lua transitions
from a rather small input. A single 16Kb URL took ~0.6s of CPU.
Consult Lua at most twice per user field instead: once when the limit is
first exceeded, and once again when the field ends at ':' or '@' so that
custom filters still see the complete userinfo. The built-in checks
cannot change their verdict in between, as the C parser bails out on any
non-printable byte in this state.
Parsing results (host, user field, obscured/has_user flags) are
unchanged; the same 16Kb URL now takes ~0.001s.
[Fix] regexp: bound the heap used by a single match
PCRE2 match contexts set only the backtracking and depth limits, leaving
the heap limit at the PCRE2 build time default (PCRE2_HEAP_LIMIT, 20 GB),
which is effectively no limit. The interpreter grows its backtracking
frames vector as `framesize * depth`, so one match on a 60 KB subject with
a 2 KB frame could allocate ~485 MB before the depth limit stopped it, and
far more with a larger frame. This binds the non-JIT path, taken whenever
JIT is unavailable or has been refused for a pattern.
Set an explicit 64 MiB heap limit, and prefer pcre2_set_depth_limit over
the deprecated pcre2_set_recursion_limit where available, falling back to
the old name for PCRE2 < 10.30.
The PCRE1 path had no explicit limits at all: they live in the pcre_extra
block, and pcre_study legitimately returns NULL when there is nothing to
optimise, so allocate one when needed and set match_limit along with
match_limit_recursion. PCRE1 backtracks on the C stack, hence a much lower
depth limit there.
While at it, in the PCRE1 path:
* free the study data unconditionally instead of only in JIT builds;
* study raw_re rather than re when building raw_extra, as the study data
is pattern specific and was being passed to pcre_exec for a different
compiled pattern.
[Fix] re_cache: propagate the regexp data limit to named scopes
`regexp.max_size` was applied to the list head only, so regexps registered
in a named scope (e.g. multimap `regexp_rules`) were matched against
unbounded input whilst the default scope was capped at 1 MiB.
Named scope caches are now created inheriting the default scope limit, and
setting the limit updates every scope registered so far, which covers both
orderings: Lua scopes registered before the regexp module is configured and
plugins that register their scopes during filters init.
[Fix] regexp: stop matchn from looping on empty matches
rspamd_regexp_search() resumes an incremental search from *end, so a
zero-width match leaves the cursor where it was. re:search(), re:split()
and the re_cache loop all break out on `start >= end` for exactly this
reason, but re:matchn() had no such guard: with a negative max_matches
(unlimited) an empty-matchable pattern spins forever and hangs the
worker.
This is reachable from configuration: sa_regexp_match() in the
spamassassin plugin passes -1 for any `multiple` rule that does not set
`maxhits`, so an SA rule whose pattern can match an empty string wedges
the process.
Add the same `start >= end` break, after the max_matches check to match
the ordering used in re_cache.c.
[CritFix] regexp: do not leak PCRE2 match data on invalid UTF input
rspamd_regexp_search() allocates the match data before the JIT UTF-8
validation guard, but the early return on invalid input skipped the
pcre2_match_data_free() at the function tail. Every rejected input
leaked one match data block sized ncaptures + 1.
The guard tests re->re != re->raw_re, which is true for any ordinary
regexp (a distinct raw_re is compiled unless the pattern is explicitly
raw or explicitly UTF). It does not test whether raw matching was
actually selected, so passing raw = TRUE does not avoid it. re_cache
routes is_raw && re_class->has_utf8 to PCRE deliberately, so a message
with a non-UTF-8 text part reaches this path through the normal scan
pipeline, once per such regexp per part.
[Fix] regexp: do not read past the end of a bounded pattern
rspamd_regexp_new_len() takes a pointer and a length, and parses
within those bounds, but rspamd_regexp_generate_id() then called
strlen() on the very same pointer. For callers that pass a slice of a
larger buffer (the composites expression parser and the symcache
delayed symbols) this reads past the end of the pattern: an ASAN build
reports a heap-buffer-overflow whenever the slice is not NUL
terminated.
Even when the read stays inside an enclosing C string the identity is
wrong, since the trailing bytes are hashed as part of the pattern: the
same regexp taken as a slice and as a standalone string got different
ids, while two different slices sharing a start address got the same
id and hence collided in the regexp cache, whose hash and equality
functions compare ids only.
Pass the length down to rspamd_regexp_generate_id() and bound the
three error paths that printed the raw pointer with %s. Note that
g_set_error() goes through the system printf and needs %.*s, while
msg_warn() uses the rspamd printf and needs %*s.
Ids produced by rspamd_regexp_new() and the regexp cache are
unchanged.
[Feature] WebUI: read-only access to selectors tab
Open list_extractors, list_transforms and check_selector to
read-only users (enable = false, matching maps.lua). These only
return static catalogs or parse the selector without executing it.
check_message stays privileged (enable = true): it runs a
user-supplied selector against a user-supplied message, so it
remains denied to read-only users (HTTP 401)
[Fix] tokenizer: bound the words retained for a message
The UTF tokenizer marked the decayed words as skipped but still added
them to the words vector, unlike the raw tokenizer which drops them. The
decay therefore bounded nothing: every word of a text part was retained
(72 bytes each) and then normalized and stemmed in
rspamd_mime_part_create_words, with several task pool allocations per
word. For a single 5 MB text part of 500k words that is 34 MiB of word
vectors and 27 MiB of pool allocations, where the decay is meant to keep
around 1200 words.
The 200 ms tokenization guard did not help either, as it is only enabled
for the parts above 1 MiB, while the remaining bound is 1 GiB of words
per part. A message of many sub-megabyte parts could thus accumulate
huge word vectors within the 50 MiB message limit.
- drop the decayed words in the UTF tokenizer as the raw one does
- add a message wide budget (100k words, 1 MiB of text) shared by all
text parts and the meta words, so that many small parts cannot bypass
the per part limits; the parts that hit it are flagged with
RSPAMD_MIME_TEXT_PART_FLAG_WORDS_TRUNCATED and the message is logged
once
- enable the time checks by the number of the produced words as well as
by the part size
- skip normalizing and stemming the words that are flagged as skipped
Note that the decayed words of the UTF parts are no longer visible to
the word regexp classes and to the part similarity hashes, which matches
the long standing behaviour of the raw tokenizer.
[Fix] css: skip comments iteratively in the tokeniser
next_token() called itself after each comment, so an input made of many
sequential comments consumed stack proportionally to their count. The
parser's own max_rec limit does not cover this recursion, and the call is
only elided when the compiler applies the tail call: a debug/ASan object
keeps a real self-call and overflows the stack.
Convert the main tokenisation loop to a while loop and continue from the
offset set by consume_comment() instead of recursing. Every other branch
of the switch returns, and consume_comment() never moves the offset
backwards, so the loop always progresses.
Add doctest cases covering comment handling (leading, trailing,
interleaved, nested and unterminated comments) plus a stress case with
100k sequential comments.
[Test] ci: build and test the jemalloc configuration
rspamd ships jemalloc-enabled packages -- debian/rules turns it on for
every architecture but arm64, rpm/rspamd.spec turns it on everywhere --
yet no workflow ever passed -DENABLE_JEMALLOC. The only mention under
.github was openbsd_build.yml setting it OFF. jemalloc replaces the
allocator for the whole process, so that is a large untested surface,
and #6153 is what it let through: a startup segfault reachable only in
a jemalloc build.
Add an enable_jemalloc input to both reusable workflows and set it
explicitly per job so each one states which package it mirrors.
ubuntu_amd64 turns it on. That is the shipped Debian/Ubuntu amd64
configuration and the only job running the functional suite, and the
binary it uploads is what webui-e2e-playwright then exercises.
ubuntu-focal turns it on too, covering the gcc-10 floor. Neither needs
an image change: rspamd-build-docker installs libjemalloc-dev in the
amd64 branch of both Dockerfiles.
ubuntu_arm64 stays off deliberately, matching debian/rules, which also
disables jemalloc there; the arm64 image takes the vectorscan branch
and skips libjemalloc-dev anyway.
fedora and centos-8/9 stay off for now with a note. Their packages do
build with jemalloc, but the images install the runtime library only
(centos) or nothing at all (fedora), so enabling them needs
jemalloc-devel added in rspamd/rspamd-build-docker first.
The /errors endpoint and its WebUI table were gated behind the
enable (privileged) password. Lower the privilege check so any
authenticated user can view them.
This is safe: the endpoint returns only Rspamd's internal
operational error log (timestamp, pid, level, module, message) —
no email content, PII, or secrets — and read-only users already
see strictly more sensitive data in the History tab (sender/
recipient IPs, scores, symbols). Authentication remains required
and the endpoint is purely informational (no mutation).
[Rework] cmake: link a single shared jemalloc instance
ProcessPackage searched for "jemalloc_pic" before "jemalloc", so
find_library preferred Debian's static libjemalloc_pic.a over
libjemalloc.so, and the result landed in RSPAMD_REQUIRED_LIBRARIES,
a list linked into nine shared objects. Each of them ended up with a
complete private allocator holding its own arenas and extent map, so
memory obtained through one copy could not be released through
another.
Search for the shared library only, fail the configure step when just
a static archive is available (except under ENABLE_STATIC, which has
no shared objects at all), and attach jemalloc per target instead:
librspamd-server, the only code calling mallctl(), plus each
executable. Executables need it directly because the dynamic linker
builds its global lookup scope breadth-first, and reaching libjemalloc
only through librspamd-server would let libc win the lookup for
malloc(). --as-needed would drop the entry outright, since no rspamd
binary references a jemalloc symbol of its own, so pin it with
--push-state,--no-as-needed where the linker supports that.
malloc_conf moves to libserver/allocator_conf.c. An executable exports
nothing without -rdynamic, so with a shared libjemalloc the tuning
would have been lost silently; a shared library always exports its
symbols, and it now applies to every rspamd binary rather than just
the daemon. Ordering matters here too: the linker takes the first
definition in scope regardless of weak versus global binding, so
librspamd-server has to precede libjemalloc.
__asan_default_options lowers detect_odr_violation to 1. Constants
with vague linkage such as std::piecewise_construct legitimately
appear in more than one of our shared objects, and the default level
aborts the ASAN build from a library constructor before main() runs.
It lives in the executables because libasan carries its own weak
definition and only scope position 0 is guaranteed to win.
Also declare libjemalloc-dev in debian/control, which debian/rules has
been requiring unconditionally on non-arm64 already.
[Fix] actrie: release GLib allocations with g_free
acism_create() allocates troot, v1, v2, usev and sv with g_malloc0/g_realloc
but released them with libc free(). That only works while both resolve to the
same allocator.
With jemalloc enabled they do not. Every rspamd shared object used to embed a
private copy of the static libjemalloc_pic.a, so free() inside
librspamd-actrie ran that copy's je_free_default() on a pointer owned by
whichever jemalloc glib had bound to, and rspamd segfaulted inside
rspamd_url_init() during startup.
Keeping the whole lifecycle inside GLib makes the pair resolve to one
allocator no matter how symbol interposition falls out, which is the same
reasoning behind rspamd_getline_free().
Vendored code is exempted from clang-format as well: the hook reformats only
the lines a patch touches, which would have left this file with three tabbed
lines in an otherwise space-indented file.
[Fix] fuzzy: check admission before parsing UDP commands
Every datagram used to be parsed, and decrypted when encrypted, before
any admission check ran: the blocklist and the rate limit were only
consulted from rspamd_fuzzy_process_command, well after a session
allocation, an ECDH, a MAC verification and the Lua pre handlers.
- Check the blocklist in the UDP read loop, before allocating a
session. It only needs the source address, so a blocked peer now
costs one radix lookup. Nothing is sent back, since building a reply
would require the parse being avoided; accept_tcp_socket already
drops blocked peers without replying, so the two transports now
behave alike.
- Log a failed key lookup or MAC check at debug level rather than
error. Both are reachable by anyone who can send us a datagram,
before any rate limit applies, so an error level line per packet
turned a spoofed source flood into a log volume attack.
- Extend the per source rate limit from FUZZY_CHECK to PING and STAT,
which are answered unauthenticated and were previously unmetered. A
rate limited PING or STAT is dropped rather than answered with 403:
the reply is the entire cost of those commands, so replying anyway
would leave egress unchanged. This stays inert unless ratelimit_rate
and ratelimit_burst are set, and the existing whitelist and local
address exemptions still apply.
- Skip the ratelimit bucket allocation when rate or burst are unset.
The NaN short circuit lives in the callee, which is only reached
once a bucket exists, so every new masked source still allocated an
LRU entry that could never limit anything.
Report blocked_requests, decrypt_errors and ratelimited_requests in
fuzzystat so the drops that are now silent stay observable.
Also document why errors_ips must remain telemetry: it is incremented
when parsing failed, which takes no key and no handshake, so on UDP the
recorded address is forgeable and banning on it would let anyone
silence an arbitrary third party.
[Fix] fuzzy: fix stack overread in sqlite backend id
rspamd_snprintf's %xs treats its argument as a NUL terminated string and
calls strlen on it, but hash_out is a raw 64 byte digest with no
terminator. Building the backend id therefore read past the end of the
stack buffer, which ASAN reports as a stack-buffer-overflow on every
sqlite backend open.
Use %*xs with an explicit length, as re_cache.c and hs_helper.c already
do for the same kind of raw digest.
[CritFix] fuzzy: release TCP session ownership exactly once
Every terminal condition on a fuzzy TCP connection - EOF, read error,
invalid frame length and the timeout - simply did REF_RELEASE on the
session. That is not enough: each in-flight command retains the session
too, so with commands outstanding the refcount stayed nonzero, the
destructor did not run and both watchers remained armed. EOF is
permanently readable, so the next poll re-entered the I/O handler and
released the owner reference a second time, freeing the session while
the command callbacks still pointed at it. The timeout had the same
defect, being configured as a repeating timer.
Introduce rspamd_fuzzy_tcp_session_close as the single termination path:
guarded by a new `closed` flag, it stops the I/O watcher and the timer,
closes the descriptor and poisons it, and drops the owner reference
exactly once. Late replies for a closed session are now discarded by
rspamd_fuzzy_tcp_enqueue_reply instead of resurrecting I/O on a dead
connection.
While here, let rspamd_fuzzy_tcp_write_reply distinguish a retryable
short write from an unrecoverable one. The bool return conflated them,
so a fatal write error left EV_WRITE armed on a permanently writable
error condition and spun forever; such a session is now closed.
Reproduced under ASAN as a heap-use-after-free in
rspamd_fuzzy_tcp_enqueue_reply, freed by fuzzy_tcp_session_destroy from
rspamd_fuzzy_tcp_io, by aborting a connection while its backend lookup
was still pending.
[Fix] url_suspect: drop dead branch in user field check
Both arms of the length_thresholds.suspicious branch inserted the
very same URL_USER_PASSWORD finding, so the threshold never affected
anything. Remove the branch together with the now unused setting.
Reject mailto URLs explicitly in the same check: every email address
carries a local part, so the presence of a user is meaningless there.
The mailto parser does not set has_user today, but that intent belongs
in the check rather than in a parser detail that may change.
Cover both cases functionally: a user field without a password must
still fire, email addresses must stay silent.
[Fix] spf: return permerror when a DNS limit is hit
Exceeding `max_dns_requests` or `max_dns_nesting` only made the
offending element unparsed, so the record was still evaluated using
whatever fitted in the budget and usually ended up as a definitive fail
via its trailing `-all`. RFC 7208 4.6.4 requires permerror instead, and
rightly so: the record has not been evaluated to the end, hence its
verdict is unknown rather than negative.
Set `permfail` on both limits, as is already done for the address
lookups spawned by `mx` and `ptr`. Records carrying any flag are not put
into the LRU cache, so a permerror is not stored for the ttl of the
record.
Note that a strictly sequential evaluator could still return pass for a
record whose match precedes the term that exceeds the limit. Rspamd
resolves a record as a whole before matching, so it cannot tell that the
budget would have been enough, and reports permerror for the record.
`parse_spf_exists` picked the current record as the last element of the
resolved array, which only holds while nothing else has appended an
element of its own. An `include` earlier in the same record does exactly
that, so from then on every `exists` in that record was attached to the
element belonging to the include.
Pass the element down from `spf_process_element` as the other mechanisms
already do. The domain spec of an `exists` is expanded before this
point, hence the visible effect so far was limited to a wrong domain in
the diagnostics of an unresolvable `exists`.
Cover the mechanism itself as well, it had no functional tests at all.
[Fix] spf: enforce the include/redirect nesting limit
`rec->nested` was checked but never incremented, so `max_dns_nesting`
had no effect at all and an include chain was only ever bounded by the
DNS requests counter. That counter was compared with `>` while being
incremented after the check, hence 31 DNS elements were evaluated with
a limit of 30.
Store the depth per resolved element, as the resolution is asynchronous
and there is no call stack reflecting the position in the tree, and
check it when an include or a redirect is about to be followed. Compare
the DNS requests counter with `>=` so that exactly `max_dns_requests`
elements are evaluated.
Flattening a record recurses over the same chain, so it is also bounded
by `SPF_MAX_NESTING_HARD`, which applies even when the configurable
limits are relaxed or disabled by setting them to zero.
[Fix] spf: bound address lookups spawned by mx/ptr expansion
Names returned by mx and ptr replies were expanded to A/AAAA requests
while checking a DNS counter that never changed, and SPF uses the forced
resolver API that bypasses dns_max_requests, so a single large RRset
produced an unbounded number of queries.
Enforce both limits from RFC 7208 4.6.4 (10 address lookups per mx/ptr
element) and a new per record budget for all expansions, configurable as
`max_dns_expansions` (100 by default). Exceeding them for mx, or running
out of a record wide budget, now yields permerror for the whole record
instead of silently dropping the element, which used to end up as
R_SPF_FAIL via `-all`; extra ptr names are ignored as the RFC requires.
[Fix] dkim: bound public key size and modulus width
rspamd_dkim_make_key() took the p= tag of a DNS TXT record, checked only
that it held at least three bytes, then allocated two buffers of that
size, base64 decoded into one of them and handed the result to
d2i_PUBKEY_bio(). Nothing bounded the length or the resulting key.
A TXT RDATA field is uint16 sized and rdns falls back to TCP on a
truncated reply, so a zone can serve roughly 64k of key material per
selector. Both raw_key and keydata live as long as the key does, and the
dkim module keeps parsed keys in an LRU capped at 2048 entries by count
rather than by bytes, so a hostile zone can pin hundreds of megabytes
with one message per selector.
Cap p= at 4096 base64 characters, checked before any allocation. Real
keys are far smaller: 44 characters for ed25519, 124 for ecdsa256, 736
for 4096 bit RSA and 2784 for the 16384 bit modulus that is the widest
OpenSSL will verify against.
That length cap still admits moduli wider than OpenSSL will operate on,
and verification cost grows with the modulus - measured here at 0.10ms
for 4096 bits, 0.36ms for 8192 and 1.36ms for 16384 - so also reject
keys above 8192 bits once parsed. RFC 8301 requires verifiers to handle
1024 to 4096 bits, which leaves twice the required headroom.
Only verification is affected. Signing keys are loaded by
rspamd_dkim_sign_key_load(), and rspamd_dkim_make_key() is reached only
from rspamd_dkim_parse_key() on a DNS reply.
Three tests drive the fake resolver: a structurally valid 32768 bit key
whose p= is 5512 characters is refused on length, a real 16384 bit key
is refused on width, and a real 8192 bit key is still accepted, reaching
signature verification and failing there instead. Without this change
the first two report R_DKIM_REJECT rather than R_DKIM_PERMFAIL, since
the key was accepted.
rspamd_dkim_skip_empty_lines() scans backwards from the end of the body
looking for trailing empty lines. The got_cr, got_lf and got_crlf states
each guard their lookbehind with p >= start + 1 or p >= start + 2, but
the else arm of that guard - taken precisely when the lookbehind would
leave the slice - then went ahead and read *(p - 1) or *(p - 2) anyway.
All three are reachable from a body that is nothing but line endings:
"\r" and "\n" enter got_cr and got_lf with p == start, and "\r\n"
reaches got_crlf with p == start + 1, since got_crlf is only ever
entered with p >= start + 1.
The reads land one byte before start. Both call sites pass the
body_start recorded by the MIME parser, which is only set when
hdr_pos > 0, so that byte is the last byte of the header/body separator
and stays inside the message allocation. This is a slice violation and
undefined pointer arithmetic rather than a confirmed allocation level
overread, but the canonicalised result did depend on a byte outside the
input.
For a message that byte is '\n', so g_ascii_isspace() was always true
and the relaxed arm always moved p to start - 1, the empty body sentinel
the function already uses. Assign that directly and drop the probe. The
simple arm never used the byte at all: the read fed an if whose body was
gated on DKIM_CANON_RELAXED.
Verified with the function extracted verbatim into an ASAN harness that
places the slice at the end of an exact sized allocation: the three
cases above report a heap underread before the change and are clean
after. A differential run over every body up to four bytes drawn from
CR, LF, space, tab and 'a', with a real separator ahead of the slice,
gives identical results for both canonicalisation types across all 1560
cases, so no body hash changes.
[Fix] dkim: apply max_sigs before work and count every signature
The per message signature limit in dkim_symbol_callback() was tested at
the bottom of the loop body, after rspamd_get_dkim_key() had already
started the DNS request for the current signature, so the request that
the limit was meant to prevent was in flight before the loop broke.
The test also incremented first and compared with >, so the default of
five permitted six signatures through the entire path.
Worse, the three continue paths above it - parse failure, trusted_only
skip, and rspamd_get_dkim_key() returning FALSE - all jumped over the
increment, so signatures that took those paths never counted. Each such
header still costs a pool allocation, a full signature parse, a node on
the result list that the three DL_FOREACH passes in dkim_module_check()
walk, and an unthrottled msg_info_task line. max_mime_headers is 100000,
so that was the real bound rather than max_sigs.
Move the check to the top of the loop, before any work is done for the
header, and count every signature header that is examined. No continue
path can skip it now, and >= against a pre increment count makes the
default permit exactly five. The "stopped after N signatures" line no
longer fires when a message carries exactly max_sigs signatures, since
the loop simply ends. The count is unsigned, so the format specifier
becomes %ud.
This shifts the meaning of max_sigs from "signatures whose key we looked
up" to "signature headers examined": five junk signatures ahead of a
valid one now push the valid one out. That is not a new weakness, since
syntactically valid but unresolvable signatures already counted, so the
same displacement was already reachable with cheap parseable decoys.
Tests cover both sides of the bound, built from the known good unknown
tags message so the real signature stays byte exact: five malformed
signatures ahead of it suppress R_DKIM_ALLOW, while four - exactly at
the limit - still let it through. The first case fails without this
change, since malformed signatures used not to count.
[Fix] dkim: bound the number of h= header list items
rspamd_dkim_parse_hdrlist_common() counted every token in h=, then
preallocated that many pointers with g_ptr_array_sized_new() and, for
each non empty token, allocated a name string and a rspamd_dkim_header
from the task pool. Nothing bounded the item count, so a size limited
message expanded into a much larger allocation: roughly 32 bytes per
two input bytes for a dense a:a:a list.
The counting loop and the allocation loop use different predicates -
the former counts every colon, the latter skips empty tokens - so an h=
made only of colons reached the full preallocation while creating no
items at all, which is the cheapest form of the amplification.
The item count also drives the header selection loop in
rspamd_dkim_check(), where each entry costs a hash lookup plus a
duplicate walk bounded only per entry by max_list_iters, so a long list
combined with many duplicate headers multiplied out.
Cap the list at 1000 items, checked before any allocation. The shipped
default_sign_headers list has 27 entries, so this leaves ample room for
real signers while removing the amplification. Both parsing directions
share this function, so signing is bounded too.
The error string uses %u rather than the rspamd printf %ud, since
g_set_error() formats through GLib.
Tests cover both sides of the bound: a 1501 item list is rejected at
parse time, and a 900 item list still parses and reaches the key lookup.
[Rework] dkim: scope HAVE_ED25519 to OpenSSL key parsing
All Ed25519 crypto in the DKIM path is libsodium, not OpenSSL: verify
goes through rspamd_cryptobox_verify() to crypto_sign_verify_detached(),
signing through rspamd_cryptobox_sign(), key sizes come from
crypto_sign_bytes()/crypto_sign_secretkeybytes(), and the public/private
match is a raw memcmp of the sodium key layout. key_eddsa is a byte
buffer, not an EVP_PKEY. libsodium is a mandatory dependency: its
ProcessPackage() entry carries no OPTIONAL flag and cryptobox asserts on
sodium_init().
HAVE_ED25519 nevertheless probes OpenSSL for EVP_PKEY_ED25519 and gated
that libsodium code, including the whole verification path. An OpenSSL
build without Ed25519 therefore refused to verify ed25519 signatures
even though the mandatory libsodium was fully capable of it.
Exactly one site needs the probe: unwrapping a PEM/DER private key,
which OpenSSL parses and which requires the EVP_PKEY_ED25519 NID plus
EVP_PKEY_get_raw_private_key(). Keep the guard there and drop it from
the nine libsodium only sites; sodium.h is included unconditionally by
cryptobox.h, so none of them ever needed it. Verifier public keys
arrive as raw bytes from DNS and never touch OpenSSL.
The remaining #ifndef message no longer claims ed25519 is unsupported
outright, since that was only ever true for signing.
[CritFix] dkim: bound bh= length before body hash comparison
The bh= length validation in rspamd_create_dkim_context() covered RSA
and ECDSA but omitted DKIM_SIGN_EDDSASHA256, so an ed25519 signature
carried a fully attacker controlled ctx->bhlen: parse_bodyhash() sizes
ctx->bh from the base64 input and stores whatever length decoding
yields.
rspamd_dkim_check() then passes that length to memcmp() against buffers
that are always EVP_MAX_MD_SIZE - raw_digest on the stack and the cached
digest_normal/digest_cr/digest_crlf allocations - reading out of bounds
once bh= decodes to more than 64 bytes. The comparison runs before the
key type and signature algorithm compatibility checks, so a mismatched
key record does not prevent it; an attacker only needs a parseable DKIM
key at a domain and selector under their control. As the overread scales
with the header size it can walk off the stack, not merely trip a
sanitizer.
Add EDDSA to the parse time length check, and guard the comparison site
with an algorithm agnostic ctx->bhlen != dlen test so a future algorithm
cannot reopen the same hole by missing a parse time case. The guard
rejects nothing legitimate: for every accepted algorithm the parse time
check already forces bhlen to equal the digest size selected in the same
function.
#rrd-total-value is an integral of the message rate (truncated per
series and summed over the 6 action series), an approximation that can
overshoot the messages scanned near row boundaries. CI observed the
total reach base+4 while only +2 were actually scanned (the exact
"Scanned" counter test still passed), exceeding the previous +3 cap.
Allow +4.
[Fix] ragel: bound nested comment depth in header parsers
The Content-Disposition and SMTP date grammars parse RFC 5322 nested
comments via `fcall`, which pushes one entry onto a heap allocated
state stack per nesting level. The stack had no depth limit and grew
through realloc, so a single header could allocate memory proportional
to its own length: peak RSS scaled linearly at roughly 12 bytes per
input byte, with unbalanced opening parens as the worst case since
every byte adds a level. Bounded only by max_message (50Mb by default),
a crafted header could push a worker to around 530Mb of transient
allocation, and realloc failure hit a g_assert and aborted the process
instead of failing the parse.
Cap the nesting depth at 8 (real headers never nest more than a couple
of levels) and use fbreak to drop into the error state once it is
exceeded, so the parser stops after a fixed number of bytes rather than
consuming the whole header. Replace the g_assert with a realloc failure
path that keeps the old allocation so it can still be freed.
Note that src/ragel/content_type.rl carries the same prepush but is not
built: it is not an input to any ragel_target, and the live content type
parser is the hand written state machine in content_type.c.
`Shm-Offset` and `Shm-Length` were each checked against the segment
size on their own, never together, so an offset near the end combined
with a full length passed both checks and produced a body running past
the mapping: SIGBUS on a page-multiple segment, adjacent memory in the
scan result otherwise. Both loaders had their own copy of the code and
hence of the bug.
On platforms without POSIX shared memory the segment name is an
ordinary path, and open(2) on a name that happens to be a fifo blocks
the whole worker until somebody opens the writing end. Open with
O_NONBLOCK and require a regular object; the same applies to the
`File:` branch, which also had a stat/open race.
Both protocol versions now share one validated helper that rejects
empty, over-long and control-character names, non-numeric or
overflowing offsets and lengths (the rspamd_strtoul() result used to be
discarded), non-regular and empty segments, offset+length beyond the
segment and payloads over max_message - the very same limit the HTTP
router applies to an inline body. An offset without a length now means
"up to the end of the segment" rather than "the whole segment".
The v3 metadata path additionally rejects a negative shm_offset or
shm_length instead of printing it as a huge unsigned value, and no
longer leaks an fstring plus an ftok per request.
The proxy cleared RSPAMD_HTTP_FLAG_SHMEM by hand before installing a
new body. The following rspamd_http_message_set_body() then cleaned up
the storage as if it were an fstring and free(3)'d the refcounted
rspamd_storage_shmem the union aliases, whilst the segment descriptor
and its mapping were leaked. Add rspamd_http_message_drop_shared_body()
which releases the shared storage whilst the flags still describe it,
and use it in all three places.
storage_cleanup() skipped a segment sitting on descriptor 0, leaking
both the descriptor and the mapping whenever the kernel handed us that
one. Releasing it uncovered the reason it looked harmless: a message
can carry the shared flag before a segment exists for it (a body-less
copy inherits the flag from its origin), and the zero then read as a
valid descriptor and closed whatever occupied slot 0 - in a proxy
worker that is the accepted client socket. Initialise shm_fd to -1 in
rspamd_http_new_message() so it is only ever valid for a real segment,
then release it properly.
Whilst here: reset `name` after releasing it so the next cleanup cannot
release it twice, drop the fstat(2) hidden inside g_assert(), reject
non-regular and empty descriptors in set_body_from_fd(), guard the
length overflow in grow_body() and validate that the body of a copied
message really fits the segment it is mapped from.
[Fix] maps: map signature file as a file, not as shmem
rspamd_map_check_file_sig() gets a filesystem path and maps the `.pub`
counterpart with rspamd_file_xmap(), but reached for the `.sig` one via
rspamd_shmem_xmap(). On any platform with POSIX shared memory that is
shm_open("/path/to/map.sig"), which the flat shmem namespace rejects
outright, so verification of a signed file backed map could never
succeed there.
rspamd_shmem_xmap() mapped whatever the name resolved to, so a fifo,
a device or a directory ended up in mmap(2) with a meaningless size,
and `*size` was left untouched on failure whilst rspamd_file_xmap(),
its file counterpart, has always reported 0 for an empty object and
(gsize)-1 for everything else. Reject non-regular and empty segments
explicitly, guard the 32 bit address space overflow, keep the failing
errno intact across close(2) and give `*size` the same meaning as in
the file variant.
rspamd_shmem_mkstemp() retried shm_open(2) forever on EEXIST: bound
the loop so a permanently colliding namespace cannot spin a worker.
[Test] functional: make log saving race-free under pabot
001_merged/__init__.robot owns rspamd and redis, and pabot splits that
directory suite into one slice per sub-suite. Every slice runs the same
parent suite teardown with the same ${SUITE_NAME}, so all of them aimed
save_run_results() at one destination: 'robot-save/Cases.001 Merged'.
The 'if not isdir: makedirs' pair there is a TOCTOU race -- when two
slices finished within microseconds of each other the loser raised
FileExistsError from the suite teardown, and robot re-marked all of that
slice's passed tests as failed (a green 100 General reported as '17
tests, 0 passed, 17 failed'). Purely timing dependent, so it flaked on
one CI job at a time.
The shared destination also meant ~30 slices copied their own rspamd.log
over the same path non-atomically, leaving one arbitrary (possibly torn)
log in the uploaded artifact instead of the failing slice's log.
Now: makedirs(exist_ok=True); suite-level saves from a directory suite
land in robot-save/<suite>/pabot-<queueindex>/, where the index is what
pabot prints as [ID:n] next to the suite name; and copies go via a
temporary plus os.replace, warning instead of raising, so saving
diagnostics can never turn a green suite red.
Leaf .robot suites keep the flat path -- 411_logging reads
robot-save/${SUITE_NAME}/rspamd.stderr back -- and plain robot runs
(the serial CI phase, local dev) are unchanged.
[Feature] fuzzy: track unkeyed clients in fuzzystat
Traffic from clients without a key (e.g. permitted by allow_update)
was invisible in the key statistics, which makes storage writers
untraceable. Track such clients under a dedicated 'unkeyed'
pseudo-key with the same aggregate and per-IP counters as regular
keys.
[Feature] fuzzy: per-hash introspection via rspamadm control fuzzyhash
rspamadm control fuzzyhash <128 hex digest> asks all fuzzy workers for
storage-side diagnostics of a specific hash: flag slots and values,
creation time, remaining ttl and, using the persisted shingle set,
slot ownership (owned/foreign/vacant out of total). Ownership directly
answers the key false-positive triage question: can this hash still
produce fuzzy matches, or has its shingle anchor decayed?
Implemented as a new inspect backend API: the redis backend gathers
everything in a single server-side script round trip; sqlite queries
the digests and shingles tables synchronously. The worker control
handler replies asynchronously from the backend callback (the control
pipe is persistent) using the same fd-attachment mechanism as
fuzzy_stat.
[Feature] fuzzy: persist shingle sets with digests in redis
Store the shingle key suffixes as the 'S' field of the digest hash on
every ADD, making each digest self-describing. This fixes two
long-standing blind spots:
- DEL: digest-only deletions (e.g. periodic deletions by a hash list)
carried no shingle information, so the digest's shingle slots were
left orphaned; now they are reconstructed from the persisted set and
removed when still owned by the deleted digest.
- REFRESH: the refresh used to touch the shingle slots of the message
that happened to trigger it (a different message by definition);
now it refreshes the slots the digest still owns and reclaims the
vacant ones, so a hash that keeps matching keeps its full shingle
anchor instead of decaying.
The sqlite backend needs no changes: its shingles table references
digests with ON DELETE CASCADE and foreign keys are enforced.
rspamadm fuzzy_hash computes the fuzzy hashes of a message per
configured rule (matching the scanner tokenization bit for bit),
optionally checks them against the storage with shingles included
(-C), and queries storages for explicit hex digests (-H), printing
found/queried digests, probability, flag, timestamp and whether the
storage confirmed the matched digest.
Add Fuzzy.Hash/Fuzzy.Queried/Fuzzy.Rule/Fuzzy.Prob/Fuzzy.Flag nested
columns (schema version 11) filled from structured fuzzy results, so
false positive investigations can start from the scanner-side journal
instead of storage forensics.
X-Rspamd-Fuzzy now uses structured fuzzy results: every matched hash
is annotated with the rule, flag, probability, the queried hash for
non-exact matches and the storage timestamp. Falls back to the legacy
fuzzy_hashes pool variable when no structured results exist.
[Feature] fuzzy_check: structured match results and diagnostics API
Every fuzzy match now produces a structured record (rule, symbol,
upstream, stored and queried digests, type, probability, score, flag,
value, hash timestamp) kept in the fuzzy_matches mempool variable and
exposed to Lua via task:get_fuzzy_results(). All queried hashes,
misses included, are recorded in the fuzzy_checked variable. The
symbol option for non-exact matches now carries both hash prefixes
(flag:found:prob:type:queried), so a match is diagnosable from a scan
report alone.
The storage sets a flag bit in the first reserved byte of the reply
when the digest field contains the digest actually resolved by the
backend; the client propagates it as the 'confirmed' field, removing
the guesswork about echoed queries when talking to legacy storages.
Add fuzzy_check.check(task, cb, rule, timeout[, hashes][, server]) Lua
API: queries a storage either with the hashes generated from the task
message (shingles included) or with an explicit list of hex digests,
reporting per-reply results including misses and error codes.
[Feature] fuzzy_check: anchor probability weight curve at the match threshold
The score multiplier for non-exact matches was sqrt(prob): a concave
curve anchored at zero. Since the storage never returns prob below the
shingle match threshold (0.5), the whole reachable range collapsed into
(~0.73 .. 1.0], so a marginal 17/32 shingle overlap scored almost as
high as an exact match; combined with high-weight deny lists this
turned weak matches into instant rejects.
Replace it with ((prob - prob_bias) / (1 - prob_bias)) ^ prob_power,
anchored at the match threshold (prob_bias 0.5). prob_power defaults to
1.0: the multiplier is the fraction of the way from the threshold to an
exact match (19/32 -> 0.19, 24/32 -> 0.50, 28/32 -> 0.75); raise it per
rule for harsher discounting of weak matches. The old zero-anchored
sqrt curve is not preserved: it was simply broken. Applies to text and
HTML hashes; small images keep their existing normalized curve.
[Fix] fuzzy_check: log rule and server for error replies
403 (rate limit), 503 (access denied) and 415 (encryption required)
replies used to insert their symbols silently; with several rules
configured there was no way to tell from the logs which rule and which
server produced the error. Log an info message with the rule name and
the upstream in both the UDP and TCP reply paths.
[Fix] cfg: warn when a module section is defined multiple times
rspamd_config_get_module_opt() returns the 'rule'/option object from the
first section only when a module section is duplicated at the top level
(e.g. a stray file in modules.d), so the remaining sections are silently
ignored and their rules are never loaded. Lua modules are not affected
as get_all_opt() flattens the whole duplicate chain.
Emit an explicit warning per configured C module so configtest and the
startup log surface the problem instead of silently dropping rules.
[Fix] lua_tcp: make connection ref release one-shot to avoid double free
A Lua callback calling close() sets the sticky LUA_TCP_FLAG_FINISHED
flag, which is then observed by more than one release site (the checks
in lua_tcp_push_data/lua_tcp_push_error, the post-helper check in
lua_tcp_handler, the fatal-error caller drops and the handlers queue
drain in lua_tcp_plan_handler_event), dropping the connection ownership
reference twice. That freed the cbd while lua_tcp_handler was still
running and crashed on the subsequent TCP_RELEASE (heap-use-after-free
on ref.refcount).
Guard the ownership drop with LUA_TCP_FLAG_FIN_RELEASED via the
lua_tcp_release_conn() helper so the first site to observe termination
wins and the rest become no-ops.
After login, #navBar loses d-none from the /auth "complete"
callback, which can fire before Bootstrap has finished the async
fade-out of #connectDialog. The lingering .show then intercepts the
immediate nav click, exhausting the 30s budget on WebKit under CI
load. Wait for the dialog to be fully hidden before interacting with
the nav, mirroring the explicit-wait pattern from 735609c1.
Resolve conflict in src/libserver/maps/map.c: take master's zstd
decompression refactor (rspamd_zstd_decompress_bounded) and re-apply
credential redaction (uri -> uri_log) in the new error paths.
[Fix] lua_content/pdf: emit newlines for Td/TD line breaks
The Td/TD text positioning operators never produced a newline: the
grammar did not capture the operator name, so the handler compared a
numeric operand against the string 'Td' (never true) and additionally
read the wrong operand. As a result consecutive text lines were
concatenated, e.g. "...June 1, 2026." + "You may obtain..." became
"2026.You", which the URL parser mis-detected as http://2026.you and
raised false positives.
Capture the Td/TD operator, split the graphics ternary operators (d/m/l)
into their own dropped branch, and render a newline when the vertical
displacement (ty) is non-zero. Adds a regression unit test.
Microsoft's JMRP feedback loop ships a well-formed
multipart/report;report-type=feedback-report whose
message/feedback-report block only carries
Feedback-Type/User-Agent/Version. Source-IP, Arrival-Date,
Original-Mail-From and Original-Rcpt-To are omitted; that data lives
only in the embedded original-message headers.
parse_arf now recovers those fields from the original headers, filling
only what the report itself left empty and recording provenance in
result.derived so callers can tell reported data from inferred data:
* source_ip: X-Originating-IP, then client-ip= of Received-SPF /
Authentication-Results, then the first public IP while walking the
Received chain (is_nonpublic_ip skips RFC1918/CGNAT/ULA hops, which
ip:is_local() does not).
* arrival_date: topmost Received timestamp, else Date.
* original_mail_from: Return-Path, else smtp.mailfrom.
* original_rcpt_to: Delivered-To / X-Delivered-To / envelope-to.
* reported_domain: falls back to the envelope-from domain.
Also accept Received-Date as an Arrival-Date alias in the report block.
extract_original_message now returns the raw header maps alongside the
parsed subset; DSN parsing is unchanged. Adds unit tests for the
Received-SPF path and the public-IP-in-Received-chain fallback.
The style dimension parser found a digit but called rspamd_strtoul from
the start of the substring, ignored its return value, and assigned the
result. For substrings longer than 22 chars rspamd_strtoul returns
before writing *value, leaving img->height/width set from an
uninitialized variable; shorter CSS parsed to zero because the substring
began with ':' or whitespace.
Parse the digit run starting at the located digit, bound the length to
that run, and only assign on a successful parse.
[Feature] lua_http: forbid_local option to block requests to local networks
rspamd_http.request() connects to whatever the URL or its DNS resolution
yields, including loopback, link-local and RFC1918 destinations. Add a
forbid_local option enforced at the single connection chokepoint (after
keepalive, upstream, numeric and DNS address selection) using
rspamd_ip_is_local_cfg, i.e. the address class check plus the
configurable local_addrs radix.
Enable it by default in url_redirector: message URLs and their redirect
targets are attacker controlled, and hop limits alone do not stop a
crafted redirect from probing cloud metadata endpoints or internal
services. Operators resolving internal redirectors can set
forbid_local = false.
[Fix] lua_http: deliver errors to coroutine callers
DNS-stage failures (resolve error, no records, connection refused) went
through lua_http_push_error, which unconditionally invoked the callback
reference; coroutine-style requests have no callback (cbref == -1), so
the error was swallowed and the yielded thread was never resumed. Resume
the thread with (err, nil) instead, mirroring the connection error path.
Synchronous failures (unparseable URL, blocked session, immediate
connection or DNS-send failure) returned a bare false, which coroutine
callers read as err = false, response = nil - a crash on response.code
for every caller checking 'if not err'. Return (err, nil) there as well.
[Fix] http: enforce read deadline when data is pending at timer expiry
The read timer is a one-shot ev_timer. When it expired while data was
already waiting in the socket, the handler drained and parsed that data;
if the message remained incomplete, the connection continued with only
the I/O watcher active and no deadline at all. Report a 408 instead,
unless the drained bytes completed the message (finished or reset into
the keepalive pool).
Add a deterministic lifecycle test driving the expiry window via the
fake clock plus a max-priority ev_check writer, covering the incomplete,
salvaged and no-data cases.
Lua HTTP client responses were unlimited unless a plugin explicitly
passed max_size, and a negative max_size was converted straight to an
unsigned gsize, silently producing an effectively unlimited cap.
Add a max_lua_http_response option (256Mb by default, 0 disables)
applied as the reply body limit whenever a request does not specify
max_size; an explicit max_size of 0 still disables the limit per
request. Reject negative max_size values with a logged error instead
of letting them wrap.
[Fix] zstd: share one bounded decompression helper across HTTP, proxy and maps
The streaming zstd decompression loop was copy-pasted in seven places
with inconsistent bounding. The proxy had no output ceiling at all: on
request/response bodies (proxy_request_decompress) and on the v3
multipart result and body parts, a zstd bomb from a client or backend
could balloon memory without limit. The task and v3 protocol paths were
bounded but enforced the limit only when the buffer filled, letting the
output overshoot max_message up to twofold, and never validated the
final length. The map shm-cache, file-cache and static paths grew their
buffers with no ceiling.
Add rspamd_zstd_decompress_bounded that validates the frame-advertised
size before any allocation, caps buffer growth strictly at the limit,
drains pending decoder output after the input is consumed and frees the
buffer on every error path. Convert all seven sites to it: max_message
bounds the task, protocol v3 and proxy paths; max_map_size bounds the
map cache and static paths (the remote HTTP map path keeps its per-map
override). Cover the helper with unit tests including advertised-size
and unknown-size zstd bombs.
[Fix] map: bound remote HTTP map sizes, compressed and decompressed
Remote HTTP maps had no size ceiling: the map client connections never
called rspamd_http_connection_set_max_size, so a map server could feed
an arbitrarily large body that is fully retained in shared memory, and
the zstd path allocated the output buffer straight from the
frame-advertised decompressed size, then doubled it without limit, so a
small zstd bomb could balloon memory indefinitely.
Add a max_map_size option (256Mb by default, 0 disables) applied as the
HTTP body limit on both map client connections, reject frames
advertising a larger decompressed size upfront and enforce the same
ceiling in the streaming growth loop. The limit can be overridden per
map with max_size in the maps { } block, like the existing timeout and
keepalive knobs.
[Fix] http: bound request bodies on controller, proxy and control sockets
The normal worker caps HTTP bodies at cfg->max_message, but the
controller router, the proxy client connection and the main control
socket never called rspamd_http_connection_set_max_size. A declared
Content-Length caused an unbounded preallocation and chunked input
could grow until the connection ended, all before routing or
authentication.
Add a max_size knob to the HTTP router applied to every accepted
connection, and set cfg->max_message as the body cap on the
controller, proxy and control sockets. Oversized requests are now
rejected with 413 (controller) or a closed connection (proxy).
[Fix] task: bound zstd decompression by max_message and plug error-path leak
The legacy compressed-message path in rspamd_task_load_message allocated
the output buffer straight from the frame-advertised decompressed size
and doubled it without any ceiling, so a zstd bomb within the HTTP
max_size limit could balloon memory far beyond max_message. The
decompression-error return also leaked the output buffer, which was
registered with the task pool only on success.
Reject frames advertising more than max_message upfront, enforce
max_message in the buffer growth loop, and free the buffer on all error
returns. Apply the same upfront clamp to the v3 multipart path and honor
max_message == 0 as unlimited there, matching the HTTP layer convention.
[Fix] url: enforce max_urls at the central insertion boundary
All URL sources (HTML links, query URLs, images, displayed URLs,
subject URLs, Lua-injected URLs) now respect the configured max_urls
limit via rspamd_url_set_add_or_increase/add_or_return, not just
plain-text callbacks. Text pre-checks fixed from > to >= so the cap
is exact. Also remove a duplicate part-url add for newly-seen text
URLs introduced in 80f7567fb.
[Fix] 7zip: bound folder count and guarantee parser progress
The folder-count limit was applied only in the internal-folders branch,
so the external branch preserved an unchecked 64-bit count. The later
kCodersUnPackSize loop's error branch then logged once per folder
without consuming input, allowing a non-progressing CPU/log loop that
the synchronous parser cannot be interrupted from by the task timeout.
- Apply the > 8192 limit immediately after reading NumFolders, before
the external/internal split.
- Reject external-folder metadata instead of accepting it with
inconsistent downstream state.
- Replace the non-progressing error branch with an early return so every
parsing-loop iteration advances input or bails out.
- Bounds-check *p (the External flag) after the varint read.
- Make the uint64 -> unsigned int folder-count narrowing explicit.
- Move both folder_nstreams tables from g_alloca/g_malloc to the task
mempool to reduce stack risk and drop manual frees.
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.
Newline normalization recorded every newline as a pointer array entry
plus a mempool exception object and a GList node, then sorted the
whole exceptions list: a newline-dense body turned a few MiB of input
into hundreds of MiB of heap and millions of allocations. Cap the
recorded newline positions at 100k per part (text normalization and
line counters are unaffected), set a part flag on truncation and
expose it as newlines_truncated in textpart:get_stats().
Map URIs may carry HTTP basic-auth userinfo ("user:pass@host").
It was logged verbatim in CRITICAL (msg_err_*) messages, which feed
the /errors ring buffer exposed via the controller, leaking credentials
to anyone reading it. Add a redacted uri_log copy (userinfo replaced
with "***@") used in error logs; the real uri is kept intact as it
serves as the "maps { <uri> {} }" configuration key and carries the
userinfo needed for the Authorization header.
[Fix] Bound alt-part linking and fasttext langdet cost
Alternative text part linking used one subtree scan per text part, so
a crafted multipart/alternative with thousands of sibling text parts
made linking quadratic (~100M sibling visits at the MIME part cap).
All searches of a task now share a single visit budget; when it is
exhausted the remaining parts are left unlinked with a warning.
Fasttext language detection fed up to 1M words into the model with no
bound on word length or on the resulting token count. As the UTF
tokenizer intentionally does not enforce max_word_len, a crafted body
made of huge unspaced words expanded into 3-4 subword ngram ids per
character: hundreds of megabytes of heap and matching vector-add work
in predict() from one message. Skip overlong words, cap the total
tokens fed to the model, and refuse OOV subword expansion in the shim
for words longer than any real dictionary entry.
[Fix] message: link plain text parts to their HTML alternative
The alternative part search compared the raw IS_TEXT_PART_HTML flag
value (0 or 4) with the boolean want_html argument (0 or 1), so a
text/plain part never matched its text/html sibling: only the
html-to-plain direction of alt_text_part was ever populated.
[Fix] html: eliminate DOM recursion to survive deeply nested messages
Convert html_append_tag_content, traverse_block_tags and
html_debug_structure from native-stack recursion to explicit heap
stacks, and maintain html_tag::depth incrementally instead of walking
the parent chain per tag. With max_tags = 8192 a message can legally
nest ~8190 elements, which overflowed 512KB worker-thread stacks.
Replace Font Awesome's "SVG with JS" framework with a local subset SVG
sprite. Delete fontawesome.min.js and solid.min.js (~900 KB of JS) and
svg-with-js.min.css, and drop the fontawesome/fontawesome_solid RequireJS
paths and shim.
app/icons.js renders the glyphs: an initial sweep plus a MutationObserver
replace <i class="fas fa-<name>"> with inline <svg class="svg-inline--fa">
<use href="img/icons.svg#fa-<name>"/></svg>, carrying over non-class
attributes (id) and modifier classes (fa-spin, spacing). setIcon() swaps
an icon in place; the theme toggle (rspamd.js) and the map-modal header
(config.js) move off class toggling and [data-fa-i2svg] selectors. Each
icon keeps its natural aspect ratio: rspamd.css sizes the box at 1em
square by default, and icons.css (generated) overrides the width of
non-square glyphs. rspamd.css also adds the fa-spin keyframes
svg-with-js.min.css used to provide.
The sprite (img/icons.svg, 35 glyphs) and icons.css are generated by
icons-build.mjs (repo root) from icons-manifest.txt; v5 aliases (e.g.
times -> xmark) resolve at build time. Font Awesome
(@fortawesome/fontawesome-free, pinned 6.6.0) is a devDependency; CI
(ci_eslint) runs "npm run check:icons" - icons-check.mjs verifies every
fa-<name> in the markup is listed in the manifest, then rebuilds and
fails on any diff (stale files or Font Awesome version drift).
A chain of bare "Content-Type: message/rfc822" wrappers recursed through
rspamd_mime_parse_message's own MESSAGE branch without ever tripping the
max_nested (64) limit. When descending into an embedded message the
parser allocates a fresh runtime (nst) and seeds nst->nesting from the
parent's value *before* incrementing, then increments the parent runtime
st which is not the one carried into the recursion. The multipart branch
and rspamd_mime_process_multipart_node bump the descended-through runtime
(and push it onto the stack so the recursive call's cleanup pops and
decrements it), but the message branch did neither, so nst->nesting
stayed at its initial value at every level and the entry guard never
fired.
The result was recursion bounded only by message size (~34 bytes per
level): deeply nested messages exhaust the worker stack (remote,
unauthenticated DoS via HTTP submission or SMTP/milter delivery), and the
per-level boundary pre-scan made even non-crashing depths quadratic in
CPU. The earlier S/MIME fix (f6536945) only covered the pkcs7-mime
re-entry path and did not address this.
Push npart onto nst->stack and bump nst->nesting in the message branch,
mirroring the multipart branch; the recursive call's existing cleanup
balances both. Bounding the depth to max_nested also caps the number of
preprocess passes, eliminating the quadratic-CPU vector.
Add a Lua unit regression test that feeds a 500-level message/rfc822
chain and asserts the parsed part count stays capped near max_nested.
[Feature] Configdump: show symcache flags in --symbol-details
Add a `flags` array (fine, empty, explicit_disable, explicit_enable,
ignore_passthrough, nostat, idempotent, mime, trivial, skip, composite,
ghost, coro) to the per-symbol details, so it is possible to check
whether a symbol requires explicit enabling; structural types are
already covered by the `type` field.
[Test] Settings: cover explicit_enable gating and implicit_allow policy
New functional test cases:
* explicit_enable symbols do not run without settings, under an
implicit-allow settings id, or with raw settings that do not list
them
* whitelist settings and task:enable_symbol() unlock them
* policy = implicit_allow keeps all symbols enabled while unlocking
listed explicit_enable symbols and disabling listed symbols, both
for a registered settings id and for raw settings
[Feature] Settings: add policy option and strict explicit_enable gating
Make symbols with the explicit_enable flag actually require explicit
enabling: previously they were executed whenever any settings were
applied to a task, as the implicit-allow settings branch in
cache_item::is_allowed never consulted the flag, and any non-null
task->settings unlocked such symbols regardless of content.
Now explicit_enable symbols run only when:
* listed in symbols_enabled of the applied settings (any policy), or
* force-enabled via task:enable_symbol()
To make it possible to combine the default-allow behaviour with
selectively enabled explicit_enable symbols and disabled symbols,
introduce a settings policy option in the apply block:
With policy = implicit_allow, symbols_enabled no longer switches the
settings to whitelist mode: all symbols stay enabled, the listed ones
are merely unlocked. The policy is also honoured for raw settings
passed via the Settings header.
Additionally, symbols listed in symbols_enabled are now always tracked
as force-enabled (previously only when the settings element was already
attached to the task, which missed the map-driven settings path where
the id is set after settings are applied).
projects / thirdparty / rspamd.git / log
