aboutsummaryrefslogtreecommitdiffstats
path: root/tests
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 /tests
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
Diffstat (limited to 'tests')
-rw-r--r--tests/test_case.py10
-rw-r--r--tests/test_cli.py7
-rw-r--r--tests/test_parse.py74
3 files changed, 90 insertions, 1 deletions
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: