[Test] fuzzy: check that nothing is shared over a plain connection
Extensions must only travel over an encrypted connection, which is what
fuzzy_rule_shares_extensions() enforces via fuzzy_rule_has_encryption():
the very predicate the send path uses to decide whether the command gets
encrypted. Pin that down with the same remote sender that receives the
full set of extensions over an encrypted rule.
[Feature] fuzzy: fill the generic PTR name bit from hfilter
The bit was written as zero because the pattern set behind it was not
reachable from here. Now that hfilter publishes its verdict, read it and
add the matching ordering dependency, so that the sender facts carry the
dynamic/residential signal without a second copy of those patterns.
The bit stays zero when hfilter or its hostname checks are disabled,
exactly as the other fields are reported as absent when their check does
not run.
[Minor] hfilter: publish the generic PTR name verdict
hfilter carries the only generic/dynamic hostname pattern set in the
tree, as an inline map that nothing outside the plugin can reach. Store
the verdict for the PTR name in the task mempool so that other modules
can reuse the patterns instead of growing a second copy of them.
Only the `hard` and `very hard` tiers of the map count as generic: those
are the dynamic, dsl, pppoe and address-in-the-name families, while the
lower weights match ambiguous words such as `user`, `peer` or `host`.
[Feature] fuzzy: send sender facts along with a check
Add RSPAMD_FUZZY_EXT_SENDER_FACTS to the commands of the rules that
share extensions: one 32 bit big endian word carrying the SPF, DKIM and
DMARC results, the PTR state, a recipient count bucket and whether the
sending client used TLS. Authentication results separate `this address
is the source` from `this address relayed or forged`, which is the
distinction an IP reputation model built from this telemetry is missing
today; the rest is nearly free on the wire.
Every field describes the sender, never the recipient nor the local
policy: the recipient count is bucketed rather than exact, and the
client's own verdict, action and score are deliberately absent since
they would create a feedback loop with the reputation list.
The values come from the checks themselves rather than from symbol
names, so the spf and dmarc modules now publish their policy result in
the task mempool for anyone to consume, and DKIM is read from the
existing check results. FUZZY_CALLBACK gains ordering dependencies on
those checks, without which the results would not be there yet by the
time the commands are built.
The generic/dynamic PTR name bit is written as zero for now: the only
pattern set for it in the tree is an inline map inside hfilter.lua and
is not reachable from here.
This must not be released before storages carrying the forward
compatible extension parser are deployed.
[Fix] fuzzy: stop sharing extensions for our own senders
fuzzy_cmd_write_extensions() shipped task->from_addr to the storage
whenever the rule did not set no_share. On a deployment that runs fuzzy
checks on outbound or authenticated submission traffic that address is
the operator's own user rather than a third party, so their users' IP
addresses ended up on the public fuzzy servers.
The extensions must describe the sender and only the sender, so omit
them entirely for authenticated tasks and for sources in our own
networks. Nothing of value is lost: such addresses are noise in an IP
reputation model and are exactly the RFC1918 and loopback values that
had to be filtered downstream anyway.
Also honour what the code already claimed and skip the extensions for
rules that talk to the storage in plain text, where they would otherwise
travel in the clear.
[Fix] fuzzy: make the wire command extensions forward compatible
The extensions TLV is not self describing: the length of an entry is
implied by its type, so rspamd_fuzzy_extensions_from_wire() could not
skip a type it did not know and rejected the whole command instead. The
client then got no fuzzy answer at all rather than merely losing the
extra field, which means adding an extension type breaks every storage
that predates it.
Establish that type bytes from 0x80 up are always followed by a length
byte and can therefore be skipped when unknown, keep rejecting unknown
types below that (those genuinely cannot be skipped) and reserve
RSPAMD_FUZZY_EXT_SENDER_FACTS with its bit layout in the new range.
The parser moves to fuzzy_wire.c where both passes share a single TLV
step, which also removes the g_assert_not_reached() in the second pass:
a malformed packet from the network must never be able to abort the
daemon. While here, decode the sender facts into ext.sender for the
lua pre/post handlers, as strings rather than raw numbers.
Nothing sends the new extension yet, so a storage carrying this change
can be deployed before any client starts producing it.
Bump @fortawesome/fontawesome-free from 6.6.0 to 7.3.1 and
regenerate the WebUI icon subset (icons.svg, icons.css). FA7
redesigns several glyphs and changes some viewBoxes, so the
non-square width rules are recomputed. Paths now ship
fill="currentColor", consistent with the existing
.svg-inline--fa rule.
[Fix] mime: restrict the euc-tw misnomers to CED output
The substitution table also resolves the charset a message declares,
so folding "euc" into euc-tw there reached far past its intent: a
message declaring charset=euc is far more likely to carry sloppy
EUC-JP or EUC-KR than EUC-TW, and the mapping turned data that used
to be left raw into text that decodes cleanly and says something
else. The IANA registry has no generic EUC charset to appeal to.
Both labels are CED inventions, so they now resolve where CED output
is turned into a charset name and nowhere else. The unit case that
covered them goes with it: it exercised the global table, which is no
longer where this belongs.
[Fix] mime: refuse a cmap that inherits from another
usecmap was treated as a marker and thrown away, so a CMap built as
overrides on top of a base returned only the overrides: every code the
base was meant to answer for silently vanished, and the surviving ones
made the result look like real text.
The base is a CMap resource that is not in the file, so it cannot be
resolved here. Refuse such a CMap instead, and let the caller fall
back to leaving the part undecoded, where the computed part guard can
see it for what it is. The same applies to the /UseCMap entry of the
stream dictionary, which the Lua side now checks.
[Fix] mime: a cmap code is its width as well as its value
Mappings were stored under the numeric code alone, so <41> and <0041>
collided even though a CMap may declare both: the byte count is what
places a code in a codespace, which code_length() already honoured
when framing the input but the tables then ignored. A one byte
mapping could therefore answer for a two byte code, or be overwritten
by one.
[Fix] lua_content: keep TJ word gaps out of the code stream
The numbers in a TJ array are positioning adjustments, not character
codes, but a large negative one was turned into a space and
concatenated with the strings around it. For a simple font that only
made an ASCII space arrive by the wrong route; for a composite font
it inserted one byte into a stream of two byte codes, so
[<0024> -250 <0030>] framed as 00 24 20 00 30 and every code after
the gap was read across a boundary.
The pieces of a text operator now travel separately and the gap is
appended structurally, where it cannot disturb the framing.
A Type0 font now resolves to its /ToUnicode CMap, which takes
precedence over /Encoding: its codes are glyph indices, so an encoding
table would only invent letters. Codes the CMap does not map produce
nothing rather than leaking a glyph index into the text.
The UTF-16 guess moves from the text operator handler to the replay,
because it cannot be made without knowing the font. A composite font
emits two byte codes with a zero in every other position, which is
exactly what that heuristic looks for, so it used to hijack the very
strings a CMap is meant to decode. It now applies only where there is
no CMap to be authoritative.
rspamd_pdf_text.cmap() parses a CMap stream and builder:set_cmap()
selects it, mirroring set_encoding() for simple fonts. A builder holds
one decoder at a time, so the two setters clear each other rather than
leaving both live.
The registry anchor that kept an encoding alive now covers either kind
of decoder, since both are plain userdata the C++ side dereferences.
A composite (Type0) font addresses glyphs by index, so its character
codes mean nothing on their own and no encoding table can recover the
text; the only thing that can is the /ToUnicode CMap the producer
embedded. Parse it.
A CMap is a small PostScript program, of which three constructs
matter: begincodespacerange gives the code widths, beginbfchar maps
single codes and beginbfrange maps runs, either onto consecutive
destinations or onto an explicit array. Destinations are UTF-16BE, so
a code may expand to several characters or to a surrogate pair.
Ranges are kept as ranges when their destinations are consecutive
single characters, which is the common case, and expanded only when
the destination is a ligature. Every table is capped, because a CMap
comes from the message: a range spanning the whole two byte space
with a multi character destination expands to at most 256 entries
rather than 65536.
The builder decodes through whichever of the two mechanisms the
current font uses, and all three of its entry points now share one
dispatch, so the escaped, hex and raw forms behave identically.
[Fix] lua_content: decode pdf text with the current font
The Tf operand was parsed only so that it would not break the grammar
and was then discarded, so nothing downstream could tell which font
had drawn a run of text. Raw character codes went out to tokenisation
and charset detection instead, where they cannot be resolved: 0xe9 is
Oslash, e-acute or Egrave depending only on the /Encoding in force.
Keep the font name, resolve each font of a page to an encoding, and
replay the whole page through one decoder. A /Differences array is
honoured, and the per encoding ligature slots come back correctly.
The text grammar now emits instructions rather than finished strings,
because decoding cannot happen until the current font is known. Per
chunk quality filtering goes with it: a page is scored as a whole,
where the statistics mean something, rather than per text operator.
[Fix] lua_content: drop the bogus pdf ligature substitution
The rewrite was applied to every rendered string regardless of the
font, and a character code has no meaning without one. Of the seven
codes it touched, 0xae and 0xaf are the fi and fl ligatures only in
StandardEncoding and 0xde, 0xdf only in MacRomanEncoding, while
0xab to 0xad are none of them anywhere. In WinAnsiEncoding, the most
common encoding of all, every one of the seven is an ordinary
letter, so German text lost each germandbls to an "fl" and each
registered sign became "fi" -- before charset detection ever saw the
bytes.
Per encoding ligature slots come back with the font aware decoder.
[Fix] lua_content: keep inline pdf /Resources dictionaries
Only the indirect form was unwrapped: a /Resources written out as an
inline dictionary, which is the common case, landed in the else branch
and was replaced by an empty table. Everything a page declares --
fonts above all -- was therefore invisible, which is why the font
extraction sitting next to this code had been commented out as
unusable.
[Feature] lua: expose the pdf glyph decoder to Lua
rspamd_pdf_text wraps the libmime unit in two classes: an encoding,
built from a base encoding name and optionally a /Differences array
in its PDF form, and a builder that accumulates a page worth of runs
in one buffer.
The builder anchors the encoding object it was last given in the Lua
registry, so a font that is still current cannot be collected out
from under it, and finish() hands the buffer over as a text and
leaves the builder ready for the next page.
[Minor] mime: reuse existing helpers in the pdf glyph unit
The first cut hand rolled things rspamd already has, and always more
narrowly:
- UTF-8 encoding -> U8_APPEND_UNSAFE from ICU
- codepoint validity -> U_IS_UNICODE_CHAR, which also rejects the
noncharacters the hand written check let through
- hex digit and hex run parsing -> g_ascii_xdigit_value and
rspamd_xstrtoul
- case insensitive compare -> rspamd_lc_cmp
text_builder::add_utf8 now validates its input with the bundled
simdutf via rspamd_fast_utf8_validate and refuses ill formed runs,
so one bad string cannot leave the whole page buffer invalid.
The generated tables hold {bytes, length} instead of a NUL terminated
pointer, which drops a strlen from every glyph lookup and lets the
base tables and the /Differences overrides share one type.
[Feature] mime: decode PDF simple font text properly
A string in a PDF content stream holds character codes, not
characters: byte 0xe9 is Oslash in StandardEncoding, e-acute in
WinAnsiEncoding and Egrave in MacRomanEncoding. Without the
/Encoding of the font that drew it, such a string cannot be
decoded, and running charset detection over it is guesswork.
Add a libmime unit owning the three Annex D base encodings and the
per-font /Differences overrides, plus a builder that turns a page
worth of text runs into a single UTF-8 buffer with no allocation
per run. It also unescapes literal and hex strings, so decoding is
one pass rather than a chain of Lua gsubs.
The tables are generated by utils/gen_pdf_glyphs.py from the
fontTools StandardEncoding and MacRoman glyph lists and the Adobe
Glyph List, and from cp1252 plus the documented Annex D deviations
for WinAnsiEncoding.
Composite (Type0/CID) fonts stay out of scope: their codes are
glyph indices that only a /ToUnicode CMap can resolve.
[Minor] mime: fold CED euc-tw misnomers to a real charset
CED labels EUC-TW content "CNS" and "EUC" -- its own tables mark
both as misnamed -- and ICU resolves neither spelling, so those
parts always ended up in ucnv_open failing and being kept raw.
Now that a CED-detected charset goes through
rspamd_mime_detect_charset, one substitution entry each is enough
to make them convert.
Both labels are unresolvable today, so a message that declares one
of them outright is not converted either; folding them can only
turn "no conversion" into a conversion attempt.
Pin down the BIG5-CP950 false-positive fix with two C++ suites:
- rspamd_mime_detect_charset resolves the non-IANA CED label
"big5-cp950" (and its uppercase form, as emitted by CED) to the
canonical big5; the utf-8 fast path stays intact.
- A computed text/plain; charset=utf-8 part holding invalid UTF-8 is
kept raw by rspamd_mime_text_part_maybe_convert instead of being fed
through CED and a conversion that never applies (no U_FILE_ACCESS_ERROR,
no spurious language detection). A valid UTF-8 computed part still
becomes utf.
PDF text extraction injects a synthetic text part whose bytes are not
valid UTF-8 and belong to no legacy charset. Running CED on them produced
a non-IANA name ("BIG5-CP950") that ucnv_open rejected (U_FILE_ACCESS_ERROR).
- Keep computed/extracted text parts raw instead of CED re-detection
and conversion, so undecodable glyph data is excluded from tokenization
and language detection.
- Route the CED-detected charset through rspamd_mime_detect_charset and
map "big5-cp950" to "big5" so legitimate Big5 content converts.
Stephen Ritz [Fri, 7 Aug 2026 12:36:29 +0000 (05:36 -0700)]
[Fix] lua_mime: unwind every boundary level when returning from nested parts
add_text_footer() closed a fixed number of boundaries -- one, or two in the
multipart/related special case added in #5334 -- rather than every level the
part walk had actually left behind. The number of levels to close is a property
of the message structure, so as soon as the walk returned from a nested part to
a shallower one the stack desynchronised: boundaries[#boundaries] then named an
unrelated boundary and a closing delimiter for the top-level one was emitted
into the middle of the body. Everything after that point is epilogue per
RFC 2046, so attachments remained in the byte stream but became unreachable to
any strict parser, silently and with the message growing rather than shrinking.
Replace both call sites with close_boundaries_until(), which unwinds to the
boundary being re-entered. When the target is not on the stack at all -- a
nested message/rfc822 opening a boundary of its own -- it closes exactly one
level, so that case behaves as before. This subsumes the multipart/related
special case, which is simply one instance of "more than one level to close".
Note the special case was not merely incomplete: for multipart/mixed over
multipart/related with a trailing attachment it is what emitted the premature
top-level delimiter, so that shape was broken by the narrower fix rather than
left unfixed by it.
Add unit tests for the three shapes that distinguish the behaviours: the
three-part mixed > related trigger, the mixed > alternative > related shape
from #5334, and a nested message/rfc822 bundle. Against the unpatched module
the first and third fail and the second passes; with the fix all three pass.
[Fix] Fuzzy: per-key ACL applies to delayed v1 replies
The v1 reply path gated the per-key forbidden_ids check on prob > 0.
The DELAY branch above it zeroes ts, prob and value but leaves flag
intact, so a delayed reply took the `else if (default_disabled)` branch
instead: a key with its own forbidden_ids silently lost its ACL and the
global default_forbidden_ids applied after all. That is the same
inversion fixed in the v2 path in the previous commit, just limited to
encrypted delayed replies.
Drop the prob check so the per-key set overrides the default regardless
of the match result, matching rspamd_fuzzy_flag_is_forbidden(). Outside
DELAY this changes nothing: prob == 0 means a miss, where flag is 0 too
and neither set matches.
[Fix] Fuzzy: per-key forbidden_ids replaces the default
Since 6d15756b1 ("[Fix] Filter forbidden_ids from multi-flag extra
flags in v2 reply") the v2 reply path checks the global
default_forbidden_ids first and suppresses the flag unconditionally,
letting a per-key forbidden_ids set only add further restrictions.
That inverted the semantics. A key that carries its own forbidden_ids
in its extensions REPLACES the global default: an empty
`forbidden_ids = []` parses into a non-NULL pointer to an empty set
(kh_init on any UCL_ARRAY in fuzzy_storage_keys.c) and means "nothing
is forbidden for this key". The global list is only a default for keys
that have no ACL of their own. The v1 reply path still implements this
correctly via its default_disabled fallback and is left untouched.
Consequence in production: with the worker option forbidden_ids = [8],
flag 8 was suppressed for every client including the premium keys whose
own forbidden_ids is empty, so that feed was not served at all.
Check the per-key set alone whenever the session is encrypted and the
key has one, and fall back to default_forbidden_ids otherwise.
[Fix] html: schemeless email href is not http userinfo
html_process_url() guesses a missing scheme by scanning for the first
non-alphanumeric character and deciding on that single character. Only
a leading `@` selected the mailto: prefix, so any email address whose
local part contains a dot defeated the heuristic: for
<a href="user.name@example.com">
the first non-alnum character is the dot in `user.name`, which fell
into the generic branch, kept the http:// prefix and produced
`http://user.name@example.com` with has_user set. url_suspect then
fired URL_USER_PASSWORD (2.0) on a legitimate message - nodemailer
emits quote-attribution links without a mailto: scheme.
`user@example.com` parsed correctly only because it has no dot before
the `@`.
Treat `.`, `-`, `_` and `+` as ambiguous - they are legal both in a
hostname and in an email local part - and keep scanning until a
genuinely structural character decides the prefix. `@`, `//` at
position 0 and `:` at i != 0 keep their existing semantics, so
`javascript:`/`data:` hrefs still win when the colon precedes any `@`.
Ambiguous characters are only skipped at i != 0, so a leading one (the
relative href `./page.html`) is still rejected as invalid. Reaching the
end of the string having seen only ambiguous characters reproduces the
generic branch, which is what keeps bare hostnames such as
`www.example.com` resolving to `http://www.example.com`.
The plain-text URL scanner has its own email detection and is
untouched. There is no sibling copy of this heuristic;
html_url_rewrite.cxx does not reimplement prefix guessing.
The matrix lives in test/lua/unit/get_html_urls.lua rather than
html_tests.cxx: the C++ harness calls rspamd_url_init(NULL), which
leaves the TLD list empty, and html_process_url() drops every url that
has neither a scheme nor a TLD, so no schemeless href can be asserted
there. The C++ url comparison is also changed to compare whole vectors
- it used to index the actual array with the expected array's length
and crash under ASan whenever a case mismatched, hiding the real
assertion.
Also adds a functional case: URL_USER_PASSWORD fires on the schemeless
dotted href before this change and does not after it.
The Tabulator global search (buildSearchHaystack) built its haystack
from symbol name and description only, dropping symbol options. This
regressed option-value search (e.g. asn:205640) in 4.1.4 — the values
are rendered in the symbols cell but were not part of the searchable
text. Include sym.options in the haystack, restoring the old FooTable
behavior. Covers both history and scan tables via the shared haystack.
#rrd-total-value is a rate integral (truncated per series, summed over
the 6 action series, with a msg/s↔msg/min scaleFactor switch) — an
approximation whose delta is noisy in the small-data regime (fresh
RRD). CI observed it overshoot to +4..+6 for just 2 scans (the exact
"Scanned" counter still reported +2), so an exact-value range is
unfixable by widening. Replace it with a band: the total must rise
above the baseline (regression: scans not reflected) yet stay below a
generous ceiling that a gross normalization bug (dropped scaleFactor
or unit divisor) would blow through.
The browser cache key had no version pin, so the 'latest' matrix leg
restored stale browsers forever (the key is never rewritten on a hit)
and browsers were re-downloaded every run. Pin the 'legacy' key to the
Playwright version; roll the 'latest' key weekly (ISO year-week) with a
prefix restore-key so newest revisions are still pulled regularly while
the cache stays effective within a week. Both legs keep running on
every PR/push; npx playwright install stays as-is (idempotent, skips
already-cached browsers).
Concurrency cancel-in-progress on ci_webui_e2e_playwright keyed by
github.ref cancels the previous master commit's Playwright run when two
merges land close together. GitHub's binary commit-status model then
shows a red X on that commit even though nothing failed.
Only cancel for PR/fork refs; on refs/heads/master let runs queue and
complete so master commit history stays green, while preserving
resource-saving cancellation during PR iteration.
[Fix] Close snapshot truncation race and query corruption
Two review findings.
rspamd_shmem_segment_map() mapped the selected window and copied out of
it. The backing object belongs to the client, so an ftruncate landing
between the fstat and the memcpy raised SIGBUS inside the copy and took
the worker down; the mapping was only dropped afterwards, so the earlier
regression test, which resized the object once the call had already
returned, never exercised that window at all.
Read the window straight into pool storage instead. A read cannot fault:
a concurrent truncation merely returns fewer bytes, and the payload
length now reflects what was actually read rather than what was asked
for. Short reads and EINTR are handled in the loop.
POSIX shared memory descriptors do not accept positional reads on every
platform - macOS reports ESPIPE and some BSDs ENODEV - so the mapping
path is kept as a fallback for exactly those descriptors. On Linux,
where shared memory objects live on tmpfs and HAVE_SANE_SHMEM is always
set, the read path is the one taken and the race is closed.
proxy_strip_query_args() rebuilt the url from a prefix of
u.field_data[UF_QUERY].off bytes. That offset addresses the first byte
after the '?', so the prefix already carried the delimiter and the
appended one produced '/checkv2??From=...'. Since the proxy turns query
arguments into request headers at the upstream, the first surviving
argument was then parsed as '?From' and its value silently lost. Stop
the prefix one byte short. The previous test stripped every argument
present, so nothing survived and the corruption stayed invisible.
Both regressions are now covered. A thread toggles the size of a backing
object while the snapshot is taken repeatedly, which faults on the old
code and passes on the new one, and a proxy request now carries an
ordinary query argument alongside a privileged one and asserts the
ordinary one reaches the upstream intact.
Add a C++ unit suite for the input layer: page-aligned window mapping
at awkward offsets, a small window of a large backing object,
zero-length windows mapping nothing, max_size enforcement, the
combined offset plus length overflow check, path validation (empty,
control characters, embedded NUL, url-encoded NUL, overlong), and
snapshot stability across a concurrent truncate and grow. Also pin the
semantics of the capability helpers, including that Filename is not a
privileged control. Cleanup is RAII so a failing assertion cannot leak
an object.
Add four functional suites covering the transport-derived behaviour
that cannot be reached from a unit test: v2 File/Path/Shm* and v3
file/shm metadata refused over TCP, the proxy refusing File as header
and as query argument, a unix socket retaining the capability while
the option is false, compatibility when it is true, client Shm*
headers never overriding the proxy-generated values, inline forwarding
to an upstream that may not receive shared memory, max_message
enforcement, and pending connections counting toward the scanner,
controller and proxy admission limits with counters released on
disconnect.
Each new suite has a verified negative control: flipping only the
config value makes exactly the intended tests fail and no others.
Supporting harness changes in lib/rspamd.py: an AF_UNIX HTTPConnection
so a unix-bound worker can be exercised, helpers for benign temporary
files and shared memory objects, and helpers for holding connections
open in the accepted-but-body-pending state.
Tests use only benign temporary files and shared memory objects they
create and clean up themselves.
Set allow_file_and_shm_inputs = true in the normal, controller and proxy
worker configuration, with a comment explaining what the option permits,
that it applies to TCP listeners only, that unix sockets always allow
these inputs, and that the default becomes false in the next major
release. The comment also states that a listener with this enabled must
not be exposed to untrusted networks and that authentication and
encryption do not authorize the capability.
Add a commented max_connections example to the controller and proxy.
No shipped default value is changed, so this release stays compatible.
Operator-facing documentation for the option is maintained separately on
docs.rspamd.com.
[CritFix] Gate privileged file and shm message sources
The File/Path/Shm/Shm-Offset/Shm-Length headers, their v3 metadata
equivalents (file/shm/shm_offset/shm_length) and the proxy's File query
argument make rspamd open a path or map a shared memory object that the
*client* names. They were accepted on any transport, so any client able
to reach a scanner, controller or proxy port could have an arbitrary
file readable by the rspamd user parsed and reported back to it.
Introduce a per-worker `allow_file_and_shm_inputs` boolean on the
normal, controller and rspamd_proxy workers. The capability is derived
solely from the transport a connection was accepted on:
allowed = (accepted transport is AF_UNIX) ||
allow_file_and_shm_inputs
Unix sockets allow these inputs independently of the option, since
access is already controlled by the socket's filesystem permissions.
Nothing the client sends - User-Agent, forwarded headers, query
arguments, request metadata - takes part in the decision, and neither
does authentication: a controller password, an enable_password, a
secure_ip match and an encrypted connection are all separate
capabilities and none of them unlocks filesystem or shared memory
access.
The code default and the shipped default are both true for this release;
the default becomes false in the next major release. A prominent startup
warning is emitted per TCP listener while the option is enabled,
escalated for a non-loopback listener.
Gating is applied before the named object is opened, mapped or statted:
* v2 headers are rejected at the request header choke point, so a
privileged control is never even stored on the task;
* the v3 raw-Shm request body path is gated separately, as it maps a
segment without going through rspamd_task_load_message();
* the v3 metadata keys are gated before any header is synthesised; *
rspamd_task_load_message() is the backstop for every caller; * the
proxy rejects File and Path as header and as query argument.
Two further vectors are closed. rspamd_protocol_handle_url() turns query
arguments into request headers at the upstream, so ?Path=/?Shm= in a
proxy URL reached a scanner that trusts the proxy's loopback connection;
the proxy now strips those arguments. And fuzzy_check builds its own
task for /fuzzyadd and /fuzzydel, which bypassed the controller's gate;
it now inherits the capability from the accepted transport.
Harden the inputs themselves:
* paths are validated before any syscall (empty, embedded NUL, control
characters, overlong), and a truncated path is rejected rather than
acted upon;
* file input opens first and validates via fstat, requires a regular
file, rejects unrepresentable sizes, enforces cfg->max_message before
reading, and takes a bounded snapshot handling short reads and EINTR
instead of parsing a MAP_SHARED mapping that can fault if the caller
truncates the file;
* shared memory maps only the required page-aligned window rather than
the whole backing object, maps nothing at all for a zero-length
window, and snapshots the window before unmapping, so a mutable
caller-owned object cannot fault the parser;
* Shm/Shm-Offset/Shm-Length are treated as reserved hop-by-hop headers:
the proxy strips every client-supplied instance at ingress, and the
internal triplet is now inserted after removing any existing
instances rather than appended behind them. When shared memory
transport is not permitted for a TCP upstream a bounded inline body
is forwarded instead. Encrypted and unencrypted forwarding behave
identically.
Correct the shared body mapping lifecycle. Both munmap sites derived
their length from a fresh fstat, which over-unmaps whenever the segment
grew after a copy mapped it; they now use the length captured at mmap
time. Failed ftruncate/mmap/dup/fstat operations close descriptors,
release names and leave no dangling begin pointer, and an immutable body
can no longer be routed into the ftruncate-and-remap path.
Add connection admission limits. The scanner's max_tasks counted only
requests whose complete body had arrived; it now counts accepted and
body-pending connections, is enforced with >= before any request or body
state is allocated, and the over-limit path accepts and closes instead
of returning without accepting, which used to spin on the
level-triggered listener. Accounting is made idempotent so every early
error and pre-task disconnect decrements exactly once. Controller and
proxy gain max_connections and max_connections_per_source (0 =
unlimited), enforced before pool, session, KDF, file or shared body
allocation.
Finally, RSPAMD_TASK_PROTOCOL_FLAG_LOCAL_CLIENT was set whenever the
client claimed `User-Agent: rspamc`. It is now derived from the accepted
transport and peer identity. It had no readers, so it granted nothing,
but it must not become a privilege source. For the same reason
encrypted_only no longer exempts peers via
rspamd_inet_address_is_local(), which also matches IPv6 link-local and
site-local addresses and so let any host on the segment skip the
encryption requirement.
Error replies for these rejections are genuine 400s. Reply writing folds
error codes into the 5xx range, which would have reported a client error
as an internal server error, so an opt-in flag lets a rejection emit its
real status without changing any existing error's mapping.
[Fix] controller: rate-limit auth failures per source
Verifying a controller password runs a deliberately expensive KDF
(catena costs ~25ms, and it is what `rspamadm pw` emits by default)
synchronously in the controller's event loop. The password cache keys on
the plaintext that succeeded, so a stream of distinct wrong candidates
falls through to the KDF every time. With `count = 1` for the controller
worker, roughly 40 wrong passwords per second are enough to saturate it
and stall every other request it serves.
Bound the damage with a per-source leaky bucket of failed attempts,
checked before the password logic so a throttled source costs no CPU:
sources over budget get 429 in well under a millisecond instead of
tens of milliseconds. A successful authentication clears the bucket so
a mistyped password does not leave an operator throttled, and trusted
sources (secure_ip, unix socket) return before the check and are never
throttled. Throttled requests do not add penalty, so a shared source
cannot be pushed into a permanent lockout.
Configurable via `max_auth_failures` (default 10, 0 disables) and
`auth_failure_window` (default 60s).
Also skip the second KDF when `password` and `enable_password` are
configured to the very same hash: the enable check repeats the normal
one verbatim, which doubled the cost of every wrong password.
Note this bounds a single source, not a distributed one; keys are the
real TCP peer, so behind a trusted reverse proxy all clients share one
bucket.
[Minor] Do not fetch a hyperscan database that is already installed
A map is put in the compilation queue by its read callback and announced by a
notification once the database is there, and both paths load it, so every map
update cost twice the traffic: megabytes for a map of a few thousand patterns.
Skip the load when the helper already has a database. That cannot keep a stale
one: every read allocates a helper of its own with no database at all, its
patterns never change afterwards, and a database only ever gets installed for
the content of that very helper, the load verifying the digest before it does
so.
[Fix] Make the hyperscan cache usable with a Redis backend
Redis classifies GETEX as a write command, as it modifies the key expiry, but
lua_hs_cache used it for the TTL refresh on read without setting is_write, so
the request went to `read_servers`: with a split master/replicas setup every
single cache load failed with `READONLY You can't write against a read only
replica`, leaving all maps and multipatterns on the PCRE/ACISM fallback. The
failure was easy to miss, as the check used to skip recompilation is EXISTS, a
read command that replicas serve happily: startup stayed fast whilst no worker
ever received a database.
Use GETEX only when reads and writes are served by the same servers, otherwise
read from the replicas and refresh the TTL by a separate EXPIRE against the
master. Bail out early on store and delete when no write servers are
configured, as lua_redis dereferences write_servers unconditionally, and
sanitise the configured ttl, which reached Redis through tostring() and would
mean `EXPIRE key 0` - dropping the very database just read - when set to
anything non-positive.
A nil reply is represented by the `redis{null}` sentinel: a truthy zero sized
userdata whose metatable has a function as `__index`. It was passed to
zstd_decompress, which failed the rspamd{text} check and then blew up inside
the very code that formats the type mismatch, as it indexes `__index`
expecting a table. Publish the sentinel as `rspamd_redis.null` with
lua_redis.is_null() on top, so that a missing key is told from data by
identity, and make that reporting robust: check the type of `__index`, resolve
the class names before luaL_Buffer is initialised, and ask Lua for stack space
beforehand, so the error being reported is not replaced by the one raised
whilst describing it.
Report why a hot-swap did not happen, as the reason used to be discarded and
the only visible symptom was a `No such file or directory` warning from the
local file fallback, which was attempted even for the backends that store
nothing on disk. A plain cache miss is logged as info, whilst a real failure
stays a warning. Load the databases asynchronously on worker startup as well,
so a worker started after the hs_helper broadcast does not wait for the next
one.
Drop the trailing slash from the hs_cache_dir default: every consumer appends
a separator itself, hence `dir//hash.hs` in paths and logs. That uncovers
swapped arguments in rspamd_multipattern_try_save_hs(), which printed the
separator before the directory and only produced a usable path because of that
very slash.
Finally, compile the regexp maps that were read after the fork. Their queue is
process local and hs_helper never watches maps, so it only inherits what the
main process has read before forking: a map read later on was queued in the
process that read it and nobody ever compiled it, which is not an edge case,
as rspamd_map_preload() gives up on a map unless every backend is a file or an
HTTP one with a cached copy. A three thousand pattern map served over HTTP
would therefore stay on PCRE for good.
Let the process that read the map compile it: move the draining out of
hs_helper into map_helpers, where the queue lives, and run it from the map
periodic dtor in the primary controller, skipping the entries inherited from
main, which hs_helper takes care of. The queue is walked by name, resolving
the helper afresh at every step and dropping an entry only whilst its digest
still matches, as the queue is an array that a map read can reallocate and a
reload frees the helper behind it; a round is scheduled anew when an entry had
to be left queued. Nothing is compiled twice, as the existing existence check
is kept, and nothing is fetched twice either, as the database being loaded is
remembered. Compiling only stores the database and a notification never comes
back to its sender, so it is loaded locally as well, and a map is looked up in
the cache the moment it is queued, which is how a worker picks up a database
compiled by another process or instance. That notification now carries the
digest of the compiled content instead of a name truncated to 64 bytes, which
silently never matched for a map named after an URL.
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().
Moritz Friedrich [Tue, 28 Jul 2026 07:47:57 +0000 (09:47 +0200)]
Refactor: Move request content generation into gpt_check function
The Condition option was previously expected to return both a boolean decision and the content for the LLM request. This behavior was non-obvious and only discovered through extensive debugging and code analysis (confirmed by rspamd#5857).
The condition setting now only returns a boolean for the decision (true/false) and an optional reason. Generating the request content has been moved into the gpt_check function, making the condition callback simpler to implement.
Example condition:
```
condition = <<EOD
return function(task)
local result = task:get_metric_result()
if result then
local score = result.score
if score < 2.0 or score > 8.0 then
return false, ‘outside the range’
end
end
[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.
[Feature] elastic: structured fuzzy results and message size
- `fuzzy`: replace flat `fuzzy_hashes` with per-match objects from `task:get_fuzzy_results()`
(rule, symbol, found, queried, type, prob, flag, exact); `fuzzy_nested` toggles nested type
- `size`: `task:get_size()`, requested in #6136
#6147 added `size` to the document only, but `rspamd_meta` is `dynamic: true`, so without
the mapping the type is inferred per index instead of declared.
[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.
projects / thirdparty / rspamd.git / log
