From fd8339703ccb99fccb95059996001f6734043655 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Tue, 8 Sep 2026 16:28:01 +0200 Subject: fix: take the sender domain from the address, not the display name _ADDR_DOMAIN.search() returned the first @domain anywhere in the raw header text. A display name sits before the angle brackets and is attacker-controlled, so it won. Two ways that reached a published report. The sender was misattributed: "Billing at billing@innocent.example" filed the report against a third party who sent nothing. And it defeated the structural guarantee in sender_domains(): the module reads no recipient header, but an attacker who writes the victim's own address into the display name hands it one anyway, and it came back out as a sender domain. parseaddr() parses the header grammar rather than scanning it, so a quoted display name cannot supply the address. leaky.eml's display name now carries the recipient address. The existing test_no_ioc_holds_a_recipient_address assertion catches this class; it was green before only because the fixture used a harmless domain. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019NHaqA1Rz5ybed7wFUeQbK --- abusectl/parse.py | 33 ++++++++++++++++++++++++++++++--- tests/fixtures/leaky.eml | 2 +- tests/test_parse.py | 7 +++++++ 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/abusectl/parse.py b/abusectl/parse.py index da99cc5..b1c38f5 100644 --- a/abusectl/parse.py +++ b/abusectl/parse.py @@ -38,6 +38,7 @@ import re from dataclasses import dataclass from email import policy from email.parser import BytesParser +from email.utils import parseaddr from urllib.parse import parse_qsl, urlsplit from abusectl import redact @@ -154,11 +155,37 @@ def sending_ip(raw: bytes, trusted: list[str]) -> str | None: return None -def _domain_of(header_value: str | None) -> str | None: +def _address_of(header_value: str | None) -> str | None: + """Return the addr-spec of a sender header, ignoring its display name. + + A regex scanning the raw header text is wrong here, and both ways it is + wrong reach a published report. The display name sits BEFORE the angle + brackets and is attacker-controlled, so a first-match search returns it + in preference to the real address: `"Billing at billing@innocent.example" + ` files the report against a third party + who sent nothing. + + Worse, that same first match defeats the structural guarantee in + sender_domains(). This module never reads a recipient header, but an + attacker who writes the victim's own address into the display name hands + it one anyway, and it comes back out as a sender domain. The guarantee + holds only while the address is taken structurally. + + parseaddr() parses the header grammar rather than scanning it, so a + quoted display name cannot supply the address. + """ if header_value is None: return None - match = _ADDR_DOMAIN.search(header_value) - return match.group(1).lower() if match else None + address = parseaddr(header_value)[1] + return address.lower() or None + + +def _domain_of(header_value: str | None) -> str | None: + address = _address_of(header_value) + if address is None: + return None + _, _, domain = address.partition("@") + return domain or None def sender_domains(raw: bytes) -> dict[str, str]: diff --git a/tests/fixtures/leaky.eml b/tests/fixtures/leaky.eml index 840ce6d..f98d191 100644 --- a/tests/fixtures/leaky.eml +++ b/tests/fixtures/leaky.eml @@ -5,7 +5,7 @@ Received: from sender.example.invalid (unknown [203.0.113.42]) by mx.example.org (Postfix) with ESMTP id JJJ11 for ; Tue, 8 Sep 2026 16:00:01 +0200 (CEST) Return-Path: -From: "Billing at billing@innocent.example" +From: "Billing at you@example.org" To: Subject: Confirm now Message-ID: diff --git a/tests/test_parse.py b/tests/test_parse.py index 49fc41e..de78b5a 100644 --- a/tests/test_parse.py +++ b/tests/test_parse.py @@ -108,6 +108,13 @@ class TestSenderDomains(unittest.TestCase): domains = parse.sender_domains(load("simple.eml")) self.assertNotIn("example.org", domains.values()) + def test_the_domain_comes_from_the_address_not_the_display_name(self): + # A display name is attacker-controlled and sits BEFORE the angle + # brackets, so a regex scanning the raw header finds it first. Doing + # that files the report against whoever the attacker named. + domains = parse.sender_domains(load("leaky.eml")) + self.assertEqual(domains["from"], "sender.example.invalid") + class TestAuthResults(unittest.TestCase): def test_verdicts_are_read_as_the_server_recorded_them(self): -- cgit v1.2.3