aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-09-09 19:07:08 +0200
committerDanilo M. <danix@danix.xyz>2026-09-09 19:07:08 +0200
commit75d9744a53acf091e8a2a152210647773db189d1 (patch)
tree8f4d37a52810a037cfc75e27ce9a3736dfc57714
parentb35d6e504719da97f8d2ef19b300d5d8ee45beb0 (diff)
downloadabusectl-75d9744a53acf091e8a2a152210647773db189d1.tar.gz
abusectl-75d9744a53acf091e8a2a152210647773db189d1.zip
fix: strip the envelope recipient in every shape the grammar allows
The for-clause rule anchored on the clause terminator, which is strictly stronger than RFC 5321 4.4: For sits inside Opt-info, so With, ID, Via or a CFWS comment may legitimately follow it, and its ABNF is 1*( Path / Mailbox ) where Mailbox carries no angle brackets. Only the tidiest shape was stripped. Four ordinary ones published the victim's address in the one header a report reproduces verbatim, including "for <a@b> (envelope-from <c@d>);", which is routine Exim and Sendmail output. Anchored on the address instead. The envelope SENDER survives the cut, since that is what the report is about, and the leftover whitespace, orphan separator and emptied comment are tidied because the line is published to a third party verbatim. Table-driven over all five shapes; reverting the regex fails six of nine tests in the class. Two accepted disclosures are now named in the spec's "Where the headers come from" rather than living in a test comment: our own relay's hostname, which the by and authserv-id clauses carry by construction, and attacker-controlled free text in Subject and the From display name, which the whitelist does not and should not filter. The second is asserted as a documented limit so it reads as deliberate, and the manifest assertion now bars the obfuscated you%40example.org spelling alongside the plain one. case.create() seeds an empty headers block so an unparsed case has the same shape as a parsed one. Additive, so FORMAT_VERSION is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xj1ayFRSUQ2u7cwb3S4axE
-rw-r--r--abusectl/case.py4
-rw-r--r--abusectl/parse.py39
-rw-r--r--docs/specs/2026-09-09-report.md31
-rw-r--r--tests/test_case.py10
-rw-r--r--tests/test_cli.py7
-rw-r--r--tests/test_parse.py74
6 files changed, 160 insertions, 5 deletions
diff --git a/abusectl/case.py b/abusectl/case.py
index 4c8c2db..233bc5f 100644
--- a/abusectl/case.py
+++ b/abusectl/case.py
@@ -87,6 +87,10 @@ def create(root: Path, raw: bytes) -> Case:
"auth": [],
"contacts": [],
"destinations": [],
+ # Seeded like the rest so a created-but-unparsed case has the same
+ # shape as a parsed one: report indexes this block rather than
+ # guarding every access. Additive, so FORMAT_VERSION is unchanged.
+ "headers": [],
}
save(path, manifest)
diff --git a/abusectl/parse.py b/abusectl/parse.py
index 8773c85..2217583 100644
--- a/abusectl/parse.py
+++ b/abusectl/parse.py
@@ -63,9 +63,33 @@ _MAX_REDIRECT_DEPTH = 5
# The optional "for <addr>" clause of a Received header (RFC 5321 4.4). Our
# own boundary relay writes the ENVELOPE RECIPIENT there, so the one Received
# line a report publishes carries the victim's address verbatim unless this
-# is removed. Matched up to the clause terminator rather than to end of line
-# because "for" is not always last: a timestamp follows it after the ";".
-_RECEIVED_FOR = re.compile(r"(?is)\bfor\s+<[^>]*>\s*(?=;|$)")
+# is removed.
+#
+# Anchored on the ADDRESS, not on the clause terminator. Requiring "for" to
+# be immediately followed by ";" or end of line is strictly stronger than
+# the grammar: RFC 5321 4.4 puts For inside Opt-info, so With, ID, Via or a
+# CFWS comment may legitimately follow it, and "for <a@b> (envelope-from
+# <c@d>);" is routine Exim and Sendmail output. That terminator anchor
+# stripped only the neatest shape and published the address in four
+# ordinary ones.
+#
+# Angle brackets are optional because For is 1*( Path / Mailbox ) and
+# Mailbox carries none. The address run stops at "<>;" and whitespace so the
+# match cannot swallow the rest of the header, and a trailing ")" is trimmed
+# so the clause inside a comment does not leave an orphan bracket.
+_RECEIVED_FOR = re.compile(r"(?is)\bfor\s+<?[^\s<>;]+@[^\s<>;]+>?\)?")
+
+# Left behind once a clause is cut from the middle of a line: a doubled
+# space, or a space now sitting against the ";" that ends the Opt-info. The
+# line is published verbatim to a third party, so it should not read as the
+# output of a broken tool.
+_LEFTOVER_SPACE = re.compile(r"\s{2,}")
+_ORPHAN_SEPARATOR = re.compile(r"\s+([;)])")
+
+# A comment that held nothing but the clause, e.g. "(for <a@b>)", is now an
+# empty pair of brackets or a lone opening one. Cut rather than left as
+# debris in a published line.
+_EMPTY_COMMENT = re.compile(r"\s*\(\s*\)?\s*$")
# The headers that may appear in a published report. A WHITELIST, never a
# blacklist: a blacklist means every header this parser learns to read later
@@ -193,8 +217,15 @@ def _strip_envelope_recipient(received_value: str) -> str:
The clause is optional (RFC 5321 4.4) and carries nothing a desk needs:
the report is about who SENT the message. Only the clause goes, so the
hop's own evidence, the address and the receiving server, survives.
+
+ Every shape of the clause is cut, not merely the tidy one: see
+ _RECEIVED_FOR, where anchoring on the terminator rather than on the
+ address let four ordinary shapes publish the address.
"""
- return _RECEIVED_FOR.sub("", received_value).rstrip()
+ cut = _RECEIVED_FOR.sub("", received_value)
+ cut = _LEFTOVER_SPACE.sub(" ", cut)
+ cut = _ORPHAN_SEPARATOR.sub(r"\1", cut)
+ return _EMPTY_COMMENT.sub("", cut).strip()
def report_headers(raw: bytes, trusted: list[str]) -> list[tuple[str, str]]:
diff --git a/docs/specs/2026-09-09-report.md b/docs/specs/2026-09-09-report.md
index e5ca858..12d2157 100644
--- a/docs/specs/2026-09-09-report.md
+++ b/docs/specs/2026-09-09-report.md
@@ -207,6 +207,37 @@ The cost is real and is accepted: this is a change to `parse.py`, a new
an address, and a whitelist written by hand is exactly the kind of thing a
sweep catches being wrong.
+### Two accepted disclosures, named so they are not mistaken for leaks
+
+The first property reads as an unqualified "recipient identifiers must never
+reach a report". These are the deliberate exceptions the whitelist creates,
+recorded here rather than left to be rediscovered in a test comment.
+
+**Our own receiving relay's hostname is published.** The boundary `Received`
+line names it in its `by` clause and `Authentication-Results` names it as the
+authserv-id, so `mx.example.org` travels with every report. That is the
+user's mail host, not the user's identity, and an abuse desk learns it from
+the report's own `From` regardless. It is accepted because removing it would
+mean rewriting the inside of two headers whose value to a desk is precisely
+that they are the receiving server's own verbatim words. The consequence is
+that the manifest-wide "no bare `example.org`" assertion cannot hold over the
+`headers` block; `tests/test_cli.py` narrows it there and asserts the
+ADDRESS is still absent, which is the part that matters.
+
+**Attacker-controlled free text is published unfiltered.** `Subject` and the
+`From` display name are kept deliberately, because they are what lets a desk
+recognise a campaign. An attacker who writes the recipient's address into
+one, plainly or obfuscated as `you%40example.org`, gets it published: the
+whitelist governs WHICH headers travel, never what is inside one. This is
+not fixed by filtering free text, which is the judgement-shaped problem that
+`AGENTS.md` names as the origin of every leak this project has had. The sweep
+over real mail is what covers this class, which is one more reason it is not
+optional here.
+
+The envelope recipient is NOT in this list. Our own relay writes it into the
+boundary `Received` line's optional `for` clause, and that clause is cut
+before the line is stored, in every shape the grammar allows.
+
## The reporting identity
Three config keys, all under a `[reporter]` section:
diff --git a/tests/test_case.py b/tests/test_case.py
index c6964a9..ab7eeeb 100644
--- a/tests/test_case.py
+++ b/tests/test_case.py
@@ -45,6 +45,16 @@ class TestCaseCreation(unittest.TestCase):
manifest = json.loads((created.path / "manifest.json").read_text())
self.assertEqual(manifest["format"], case.FORMAT_VERSION)
+ def test_every_block_is_seeded_present_and_empty(self):
+ # A created-but-unparsed case must have the same SHAPE as a parsed
+ # one, so a later reader indexes a block rather than guarding every
+ # access. report will read headers and would hit a KeyError.
+ created = case.create(self.root, b"x")
+ manifest = json.loads((created.path / "manifest.json").read_text())
+ for block in ("iocs", "auth", "contacts", "destinations", "headers"):
+ self.assertIn(block, manifest)
+ self.assertEqual(manifest[block], [])
+
def test_two_cases_do_not_collide(self):
a = case.create(self.root, b"one")
b = case.create(self.root, b"two")
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 425bce9..b8fffea 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -160,7 +160,12 @@ class TestParse(unittest.TestCase):
headers = manifest.pop("headers")
self.assertNotIn("example.org", json.dumps(manifest))
# And nothing shaped like an address survives in the exception.
- self.assertNotIn("@example.org", json.dumps(headers))
+ # Both spellings: you%40example.org is not a hypothetical, it is why
+ # leaky.eml exists, and docs/plans/2026-09-09-contacts.md records a
+ # From of phish@victim%40example.org.invalid.
+ blob = json.dumps(headers)
+ self.assertNotIn("@example.org", blob)
+ self.assertNotIn("you%40example.org", blob)
names = [name for name, _ in headers]
for name in ("To", "Cc", "Delivered-To", "X-Original-To"):
self.assertNotIn(name, names)
diff --git a/tests/test_parse.py b/tests/test_parse.py
index fa3526a..6e3b7a4 100644
--- a/tests/test_parse.py
+++ b/tests/test_parse.py
@@ -336,6 +336,80 @@ class ReportHeaders(unittest.TestCase):
# The rest of the hop survives; this is a cut, not a blanking.
self.assertIn("203.0.113.42", received[0])
+ def test_every_for_clause_shape_loses_the_address(self):
+ # RFC 5321 4.4 puts For inside Opt-info, so With, ID, Via or a CFWS
+ # comment may legitimately follow it, and its ABNF is
+ # 1*( Path / Mailbox ) where Mailbox carries no angle brackets.
+ # Anchoring on "for" being immediately followed by the clause
+ # terminator matched only the neatest shape and let four routine
+ # ones through, each publishing the victim's address.
+ hop = "from a.invalid (a.invalid [203.0.113.5]) by mx.example.org "
+ shapes = (
+ "for <you@example.org> (envelope-from <b@c.invalid>); Mon, 07 Sep 2026 09:12:40 +0000",
+ "for you@example.org; Mon, 07 Sep 2026 09:12:40 +0000",
+ "for <you@example.org> with ESMTP; Mon, 07 Sep 2026 09:12:40 +0000",
+ "id qq; Mon, 07 Sep 2026 09:12:40 +0000 (for <you@example.org>)",
+ "for <you@example.org>; Mon, 07 Sep 2026 09:12:40 +0000",
+ )
+ for tail in shapes:
+ with self.subTest(tail=tail):
+ stripped = parse._strip_envelope_recipient(hop + tail)
+ self.assertNotIn("you@example.org", stripped)
+ # The hop's own evidence survives: this is a cut, not a
+ # blanking, and a rule that ate the line would pass the
+ # assertion above while destroying the report.
+ self.assertIn("203.0.113.5", stripped)
+ self.assertIn("mx.example.org", stripped)
+
+ def test_stripping_leaves_no_doubled_space_or_stray_separator(self):
+ # Cosmetic in isolation, but the result is published verbatim to a
+ # third party, so a mangled line reads as a broken tool.
+ hop = ("from a.invalid (a.invalid [203.0.113.5]) by mx.example.org"
+ " for <you@example.org>; Mon, 07 Sep 2026 09:12:40 +0000")
+ stripped = parse._strip_envelope_recipient(hop)
+ self.assertNotIn(" ", stripped)
+ self.assertNotIn(" ;", stripped)
+ self.assertIn("mx.example.org; Mon", stripped)
+
+ def test_a_comment_holding_only_the_clause_leaves_no_debris(self):
+ hop = ("from a.invalid (a.invalid [203.0.113.5]) by mx.example.org"
+ " id qq; Mon, 07 Sep 2026 09:12:40 +0000 (for <you@example.org>)")
+ stripped = parse._strip_envelope_recipient(hop)
+ self.assertNotIn("you@example.org", stripped)
+ self.assertFalse(stripped.endswith("("))
+ self.assertTrue(stripped.endswith("+0000"))
+
+ def test_the_envelope_sender_comment_survives_the_cut(self):
+ # envelope-from is the SENDER, which is what the report is about, so
+ # cutting the recipient must not take it along.
+ hop = ("from a.invalid (a.invalid [203.0.113.5]) by mx.example.org"
+ " for <you@example.org> (envelope-from <bounce@sender.invalid>);"
+ " Mon, 07 Sep 2026 09:12:40 +0000")
+ stripped = parse._strip_envelope_recipient(hop)
+ self.assertNotIn("you@example.org", stripped)
+ self.assertIn("bounce@sender.invalid", stripped)
+
+ def test_the_whitelist_does_not_filter_attacker_free_text(self):
+ # A DOCUMENTED LIMIT, not a guarantee. The spec keeps Subject and the
+ # From display name knowing both are attacker-controlled free text,
+ # because they are what lets a desk recognise a campaign. An attacker
+ # who writes the recipient's own address into one, obfuscated or not,
+ # gets it published: the whitelist governs WHICH headers travel, never
+ # what is inside one.
+ #
+ # This is asserted so the limit is visible and deliberate. Do not
+ # "fix" it by filtering free text, which is the judgement-shaped
+ # problem AGENTS.md names as the source of every leak here. The
+ # sweep over real mail is what covers this class, per AGENTS.md.
+ raw = (b"Received: from a.invalid (a.invalid [203.0.113.5])"
+ b" by mx.example.org with ESMTP id X;"
+ b" Mon, 07 Sep 2026 09:12:40 +0000\r\n"
+ b"From: <phish@sender.invalid>\r\n"
+ b"Subject: Verify you%40example.org\r\n\r\nbody\r\n")
+ headers = parse.report_headers(raw, trusted=["192.0.2.0/24"])
+ subject = dict(headers)["Subject"]
+ self.assertIn("you%40example.org", subject)
+
def test_a_forged_chain_publishes_no_hop_below_the_boundary(self):
# The same job test_a_forged_chain_stops_at_the_first_untrusted_hop
# does for sending_ip(), asserted over what actually gets published: