Skip to content

Fix Received-SPF parser mishandling '=' in VERP envelope-from addresses (issues #206, #221)#300

Open
thegushi wants to merge 1 commit into
developfrom
fix/spf-received-parse-206-221
Open

Fix Received-SPF parser mishandling '=' in VERP envelope-from addresses (issues #206, #221)#300
thegushi wants to merge 1 commit into
developfrom
fix/spf-received-parse-206-221

Conversation

@thegushi
Copy link
Copy Markdown
Collaborator

Summary

The dmarcf_parse_received_spf() key=value parser treated every = as a mode switch even when already collecting a value or inside a quoted string. VERP addresses (e.g. bounces+nonce=recipient@sender.example.com) contain = in the local part, which reset the value buffer mid-parse. When the pre-= portion was longer than the post-= portion, leftover bytes corrupted the extracted domain, causing a false DMARC fail.

  • in_value flag: = is now only treated as a key→value separator once per pair; subsequent = chars are collected as part of the value.
  • Quoting guard: = and ; inside quoted strings are no longer treated as delimiters.
  • Extraction: dmarcf_parse_received_spf() moved to opendmarc-spf-parse.c so it can be unit tested without libmilter headers.

Closes #206. Related: #221. Tracked in #299.

Test plan

…es (issues #206, #221)

The key=value parser in dmarcf_parse_received_spf() treated every '='
as a switch from key-collection to value-collection mode, even when
already collecting a value or inside a quoted string. VERP addresses
embed the original recipient's address in the local part using '=' as
a separator (e.g. bounces+nonce=recipient@sender.example.com), so a
'=' in the local part reset the value buffer mid-parse.

When the pre-'=' portion was longer than the post-'=' portion, the new
write did not reach far enough to overwrite the leftover bytes, which
then corrupted the extracted domain (e.g. "sender.com" became
"sender.comnts"). The corrupted domain failed the envdomain comparison
and the SPF pass was discarded as neutral, causing a false DMARC fail.

In issue #221 (Twitter VERP with triple '==='), multiple resets
accidentally walked the parser to the correct final fragment, producing
the right answer for the wrong reason. Variants where the pre-last-'='
string is longer than the post-last-'=' string expose the same
corruption and produce wrong results with the old code.

Fix: track whether value-collection has started (in_value flag) so
that '=' is only treated as a mode-switch once per key=value pair.
Also honour the quoting flag for both '=' and ';' to handle quoted
envelope-from values correctly.

Extracted dmarcf_parse_received_spf() into opendmarc-spf-parse.c so
it can be unit tested without depending on libmilter headers.
Regression tests cover both reported cases plus variants.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-testing Item in a testing branch, feedback required to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant