Fix ReDoS in failure-report, schema-tag, and timestamp regexes (11.0.4) (#921)

Three regular expressions that run on attacker-controlled input
backtracked super-linearly (CWE-1333):

GHSA-7grx-mj6g-qxmc (affects 8.2.0 through 11.0.3, reported by
mjatin2007): text_report_regex, used by the Exim/cPanel plain-text
failure-report fallback, was \s*([a-zA-Z\s]+):\s(.+). The leading \s*
and the \s inside the class overlap and the pattern is unanchored, so a
whitespace run is retried from every offset and every split: cubic,
about 8x per doubling (400 spaces 0.06 s, 1600 spaces 3.3 s on CPython
3.12). A ~100 KB message would stall the watch loop for days and be
re-parsed after every restart. The pattern is now anchored to a line
start with disjoint classes.

GHSA-4h9r-c435-j834 (affects 4.1.7 through 11.0.3, found while auditing
every regex in the package): xml_schema_regex was </??xs:schema.*>,
whose greedy .*> rescans to end of line for every <xs:schema
occurrence: quadratic, 1 MB in 19 s. It runs on every attachment that
decodes to "<" after extract_report() has inflated it up to the 100 MiB
cap, which 194 KB of gzip reaches. Now </?xs:schema[^<>]*>. This also
fixes the greedy match deleting a minified single-line report from the
schema tag to the last ">" on the line.

human_timestamp_to_datetime() stripped RFC 5322 comments with
\s*\(.*\)\s*, quadratic on whitespace (200,000 spaces 18.6 s) and
reached through every Date header; filed as a bug fix because MTAs cap
header size. Comments are now removed by a depth-counting scan, which is
linear and handles the nested comments and quoted-pairs RFC 5322
section 3.2.2 allows (a character-class regex would have regressed
them).

Adds regression tests sized so the old patterns fail the time bound in
minutes rather than hanging CI, a "Regular expressions on untrusted
input" section in AGENTS.md, and the 11.0.4 changelog and version bump.

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>

AGENTS.md

@@ -124,6 +124,17 @@ IP address info cached for 4 hours, seen aggregate report IDs cached for 1 hour
 - Token file writes must create parent directories before opening for write.
 - Store natively numeric values as numbers, not pre-formatted strings. Example: ASN is stored as `int 15169`, not `"AS15169"`; Elasticsearch / OpenSearch mappings for such fields use `Integer()` so consumers can do range queries and numeric sorts. Display layers format with a prefix at render time.
 
+### Regular expressions on untrusted input
+
+Assume every regex in the package runs on content a remote sender controls (message bodies, headers, attachments inflated to the 100 MiB decompression cap), so a pattern that backtracks super-linearly is a remote CPU-exhaustion bug, not a style nit (GHSA-7grx-mj6g-qxmc: a cubic pattern on whitespace; GHSA-4h9r-c435-j834: a quadratic `.*>` that a 194 KB gzip attachment could turn into days of CPU). Before adding or editing a pattern:
+
+- **Adjacent quantifiers must not overlap.** `\s*([a-zA-Z\s]+)` lets both parts consume the same whitespace, so every split is retried. Make neighbouring classes disjoint (`[ \t]*` next to `[A-Za-z]`).
+- **A repeated class must exclude the delimiter it stops on.** Write `\([^()]*\)` and `<tag[^<>]*>`, never `\(.*\)` or `<tag.*>`: a greedy `.*` scans to end of line and backtracks for every occurrence, which is quadratic whenever the closer is missing, and also matches past the *first* closer (the old schema-tag regex deleted a whole minified report that way).
+- **Anchor when the match can only start at a line start** (`^` with `re.MULTILINE`); an unanchored `findall` retries the whole pattern from every offset.
+- **Prefer `str` methods** (`split`, `partition`, `startswith`) when the format is fixed and a regex adds nothing, and a plain loop when the grammar nests: balanced parentheses cannot be matched by a regex at all, which is why `human_timestamp_to_datetime()` strips RFC 5322 comments with a depth-counting scan.
+- **Time the pattern on an adversarial input before shipping it**, not just on the samples: a 1 MB string of the repeated opener with no closer, and a 200,000-character whitespace run. A linear pattern finishes in milliseconds; anything that takes seconds is a bug. Add that input as a time-bound regression test with a generous bound (seconds) and cite the measured old timing in the docstring, as the regression tests in `tests/test_init.py` and `tests/test_utils.py` do.
+- Python 3.10 is the floor, so possessive quantifiers and atomic groups (3.11+) are not available; fix the pattern's structure instead.
+
 ## Testing standards
 
 These rules govern *every* test added to `tests/`. They exist because the project has been burned by tests that looked like coverage but caught nothing, and by bug claims that turned out to be wrong about the spec. Both failure modes erode trust faster than missing coverage does.

CHANGELOG.md

@@ -1,9 +1,15 @@
 # Changelog
 
-## Unreleased
+## 11.0.4
+
+### Security
+
+- **Fixed catastrophic regex backtracking in the plain-text failure report fallback** ([GHSA-7grx-mj6g-qxmc](https://github.com/domainaware/parsedmarc/security/advisories/GHSA-7grx-mj6g-qxmc), affects 8.2.0 through 11.0.3). A `text/plain` message containing `A message claiming to be from you has failed` is parsed as an Exim/cPanel failure notice by `text_report_regex`, `\s*([a-zA-Z\s]+):\s(.+)`. Its leading `\s*` and the `\s` inside the field-name class can each consume the same whitespace, and the pattern is unanchored, so a run of whitespace is retried from every starting position and every way of splitting it: the cost grows cubically, about 8x per doubling — on CPython 3.12 (x86-64), 400 spaces took 0.06 s and 1600 spaces took 3.3 s. Anyone who can send mail to a monitored mailbox controls the body, so a message of about 100 KB would stall the mailbox watch loop for days, and since the message is never marked processed it would be re-parsed after every restart. The pattern is now anchored to the start of a line and its classes no longer overlap, so it runs in linear time. The pattern now requires a field name to start a line, with its words separated by single spaces, which is what Exim emits; notices with a tab, doubled space, or line break inside a field name were already rejected, because such names never produced the keys the parser looks up. Reported by mjatin2007.
+- **Fixed quadratic regex backtracking when stripping XML schema tags from aggregate reports** ([GHSA-4h9r-c435-j834](https://github.com/domainaware/parsedmarc/security/advisories/GHSA-4h9r-c435-j834), affects 4.1.7 through 11.0.3). `parse_aggregate_report_xml()` removed `<xs:schema>` tags with `</??xs:schema.*>`, whose unanchored greedy `.*>` scanned to the end of the line for every `<xs:schema` it found, so a line of repeated `<xs:schema` took quadratic time — 1 MB took 19 s. The pattern runs on report attachments after gzip and zip are inflated, up to the 100 MiB decompression cap, and about 194 KB of gzip inflates to that cap. The pattern now stops at the first `>` (`</?xs:schema[^<>]*>`) and runs in linear time. This also fixes a correctness bug: on a minified report with the whole document on one line, the greedy `.*>` deleted everything from the schema tag to the last `>` on the line, discarding the report.
 
 ### Bug fixes
 
+- **Parsing a timestamp no longer takes quadratic time on whitespace.** `human_timestamp_to_datetime()` stripped parenthesized comments with the regex `\s*\(.*\)\s*`, whose leading `\s*` retried every whitespace run from each starting position. The function runs on every message's `Date` header, so a long folded header was enough: 200,000 spaces took 18.6 s. This is the same weakness as the two entries above, but a lesser one: the input is a header that mail servers cap in size (Postfix's default is 100 KB, about 5 s) rather than an attachment inflated to the 100 MiB decompression cap, so it is filed as a bug fix without an advisory. Comments are now removed by a single depth-counting pass, which is linear and also handles the nested comments and quoted-pairs that RFC 5322 section 3.2.2 allows and that the old regex only stripped by accident.
 - **The Grafana dashboard's "Message sources by name and type" table no longer comes up empty when a message source has a name but no type.** Whenever a source's reverse DNS base domain is not in `base_reverse_dns_map.csv`, `get_service_from_reverse_dns_base_domain()` names the source after that base domain and leaves its type `None`; a source with no PTR record whose AS domain is not in the map is likewise named after its AS name with no type. For every such source the saved aggregate record has a `source_name` but no `source_type`. The panel nests a `terms` aggregation on `source_type.keyword` under one on `source_name.keyword`, without a `missing` value, so such a source produced a `source_name` bucket with no `source_type` sub-bucket, and Grafana's Elasticsearch data source then failed to build the table from the response. With Grafana 13.2.4 (bundled Elasticsearch data source 12.9.2) and the reports in `samples/aggregate/`, 12 sources had a name but no type and the table showed "No data"; with `missing` set, it lists all 27 name/type rows. The `source_type.keyword` aggregation now sets `missing` to `-`, the placeholder most of the dashboard's other tables already use for absent values, so those sources are listed with `-` as their type.
 
 ## 11.0.3

parsedmarc/__init__.py

@@ -88,8 +88,17 @@ logger.debug(f"parsedmarc v{__version__}")
 # than relying on ``.`` — otherwise every field value keeps a trailing CR.
 feedback_report_regex = re.compile(r"^([\w\-]+): ([^\r\n]+)\r?$", re.MULTILINE)
 xml_header_regex = re.compile(r"^<\?xml .*?>", re.MULTILINE)
-xml_schema_regex = re.compile(r"</??xs:schema.*>", re.MULTILINE)
-text_report_regex = re.compile(r"\s*([a-zA-Z\s]+):\s(.+)", re.MULTILINE)
+# Every pattern here runs on attacker-controlled input; the next two were
+# rewritten to match in linear time (GHSA-4h9r-c435-j834, GHSA-7grx-mj6g-qxmc):
+# a repeated character class excludes the delimiter it stops on, adjacent
+# quantifiers never overlap, and text_report_regex is anchored to a line start,
+# so each candidate match is scanned once instead of being rescanned from every
+# offset on every backtrack. See "Regular expressions on untrusted input" in
+# AGENTS.md.
+xml_schema_regex = re.compile(r"</?xs:schema[^<>]*>")
+text_report_regex = re.compile(
+    r"^[ \t]*([A-Za-z]+(?: [A-Za-z]+)*):[ \t]+([^\r\n]+)", re.MULTILINE
+)
 # Captures the value of any xmlns (default or prefixed) declaration so the
 # RFC 9990 namespace can be detected before xmltodict drops it.
 xml_namespace_regex = re.compile(

parsedmarc/constants.py

@@ -1,4 +1,4 @@
-__version__ = "11.0.3"
+__version__ = "11.0.4"
 
 USER_AGENT = f"parsedmarc/{__version__}"
 

parsedmarc/utils.py

@@ -12,7 +12,6 @@ import json
 import logging
 import mailbox
 import os
-import re
 import shutil
 import subprocess
 import tempfile
@@ -61,8 +60,6 @@ _RETRYABLE_DNS_ERRORS = (
 _DOH_SESSION: httpx.Client | None = None
 _DOH_SESSION_PID: int | None = None
 
-parenthesis_regex = re.compile(r"\s*\(.*\)\s*")
-
 null_file = subprocess.DEVNULL
 mailparser_logger = logging.getLogger("mailparser")
 mailparser_logger.setLevel(logging.CRITICAL)
@@ -536,6 +533,34 @@ def timestamp_to_human(timestamp: int) -> str:
     return timestamp_to_datetime(timestamp).strftime("%Y-%m-%d %H:%M:%S")
 
 
+def _strip_comments(value: str) -> str:
+    """Removes RFC 5322 comments from a header value.
+
+    A comment is parenthesized text that may nest and may contain quoted-pairs
+    such as ``\\)`` (RFC 5322 section 3.2.2), so it cannot be matched by a
+    regular expression at all. This single pass tracks the nesting depth
+    instead; an unclosed comment runs to the end of the value.
+    """
+    out: list[str] = []
+    depth = 0
+    escaped = False
+    for char in value:
+        if depth:
+            if escaped:
+                escaped = False
+            elif char == "\\":
+                escaped = True
+            elif char == "(":
+                depth += 1
+            elif char == ")":
+                depth -= 1
+        elif char == "(":
+            depth = 1
+        else:
+            out.append(char)
+    return "".join(out)
+
+
 def human_timestamp_to_datetime(
     human_timestamp: str, *, to_utc: bool = False, assume_utc: bool = False
 ) -> datetime:
@@ -556,7 +581,7 @@ def human_timestamp_to_datetime(
     """
 
     human_timestamp = human_timestamp.replace("-0000", "")
-    human_timestamp = parenthesis_regex.sub("", human_timestamp)
+    human_timestamp = _strip_comments(human_timestamp).strip()
 
     dt = parse_date(human_timestamp)
     if assume_utc and dt.tzinfo is None:

tests/test_init.py

@@ -16,6 +16,7 @@ import logging
 import mailbox
 import os
 import quopri
+import time
 import unittest
 from collections.abc import Callable
 from datetime import datetime, timedelta, timezone
@@ -2625,6 +2626,53 @@ class TestMalformedXmlRecovery(unittest.TestCase):
         self.assertIn("missing required section", str(ctx.exception))
         self.assertIsInstance(ctx.exception.__cause__, AttributeError)
 
+    def testXmlSchemaRegexRejectsRepeatedTagsQuickly(self):
+        """A report made of repeated ``<xs:schema`` is rejected in linear
+        time.
+
+        Regression test for GHSA-4h9r-c435-j834: xml_schema_regex was
+        ``</??xs:schema.*>``, whose unanchored greedy ``.*>`` rescanned to the
+        end of the line for every ``<xs:schema`` — quadratic, so 1 MB took
+        19 s and this 5 MB input would take minutes. The 10 s bound is not
+        flaky: the fixed pattern handles it in milliseconds, orders of
+        magnitude below both the bound and the old pattern's cost."""
+        start = time.perf_counter()
+        with self.assertRaises(parsedmarc.InvalidAggregateReport):
+            parsedmarc.parse_aggregate_report_xml(
+                "<xs:schema" * 500_000, offline=OFFLINE_MODE
+            )
+        self.assertLess(time.perf_counter() - start, 10)
+
+    def testMinifiedAggregateReportWithSchemaTag(self):
+        """A report minified onto one line still parses when it carries an
+        ``<xs:schema>`` tag.
+
+        The old greedy ``</??xs:schema.*>`` (GHSA-4h9r-c435-j834) deleted
+        from the schema tag to the last ``>`` on the line, which on a
+        single-line document discarded the whole report."""
+        path = "samples/aggregate/ikea.com!example.de!1538690400!1538776800.xml"
+        with open(path, encoding="utf-8") as f:
+            original = f.read()
+        minified = "".join(line.strip() for line in original.splitlines())
+        self.assertIn("<xs:schema", minified)
+        self.assertNotIn("\n", minified)
+        expected = parsedmarc.parse_aggregate_report_xml(
+            original, always_use_local_files=True, offline=OFFLINE_MODE
+        )
+        report = parsedmarc.parse_aggregate_report_xml(
+            minified, always_use_local_files=True, offline=OFFLINE_MODE
+        )
+        self.assertEqual(
+            report["report_metadata"]["org_name"],
+            expected["report_metadata"]["org_name"],
+        )
+        self.assertEqual(
+            report["report_metadata"]["report_id"],
+            expected["report_metadata"]["report_id"],
+        )
+        self.assertEqual(len(report["records"]), len(expected["records"]))
+        self.assertGreater(len(report["records"]), 0)
+
 
 class TestPolicyPublishedEdgeCases(unittest.TestCase):
     """Tests for edge cases in policy_published parsing"""
@@ -3119,6 +3167,53 @@ This is not a DMARC report."""
             parsedmarc.parse_report_email(eml, offline=True)
         self.assertIn("Tls", str(ctx.exception))
 
+    def testTextReportRegexRejectsWhitespacePaddingQuickly(self):
+        """A plain-text failure notice padded with whitespace is rejected in
+        linear time.
+
+        Regression test for GHSA-7grx-mj6g-qxmc: text_report_regex was
+        ``\\s*([a-zA-Z\\s]+):\\s(.+)``, whose overlapping, unanchored
+        whitespace quantifiers backtracked cubically — 1600 spaces took
+        3.3 s, about 8x per doubling, so this 6,400-space body takes about
+        210 s: far over the bound, but short enough that a regression fails
+        instead of hanging the CI job. The 10 s bound is not flaky:
+        the fixed pattern parses it in milliseconds, orders of magnitude
+        below both the bound and the old pattern's cost."""
+        padding = " " * 6_400
+        body = "\n".join(padding[i : i + 70] for i in range(0, len(padding), 70))
+        msg = (
+            "From: mailer-daemon@example.com\n"
+            "To: dmarc@example.com\n"
+            "Subject: Failure notice\n"
+            "Date: Mon, 01 Jan 2024 00:00:00 +0000\n"
+            "Content-Type: text/plain\n\n"
+            "A message claiming to be from you has failed\n" + body + "\ndetected."
+        )
+        start = time.perf_counter()
+        # The required received-date field is missing, so the notice is
+        # rejected rather than parsed.
+        with self.assertRaises(parsedmarc.InvalidDMARCReport):
+            parsedmarc.parse_report_email(msg, offline=OFFLINE_MODE)
+        self.assertLess(time.perf_counter() - start, 10)
+
+    def testTextReportFieldNamesMustBeSingleLine(self):
+        """text_report_regex matches Exim's one-line ``Name: value`` fields:
+        a field name cannot span a line break, leading indentation is
+        allowed, and a CRLF line ending is not kept in the value
+        (GHSA-7grx-mj6g-qxmc)."""
+        matches = parsedmarc.text_report_regex.findall("Sender\nDomain: x")
+        self.assertFalse(any("\n" in name for name, _ in matches))
+        self.assertEqual(
+            parsedmarc.text_report_regex.findall("  Sender IP Address: 1.2.3.4\n"),
+            [("Sender IP Address", "1.2.3.4")],
+        )
+        self.assertEqual(
+            parsedmarc.text_report_regex.findall(
+                "Received Date: Mon, 01 Jan 2024 00:00:00 +0000\r\n"
+            ),
+            [("Received Date", "Mon, 01 Jan 2024 00:00:00 +0000")],
+        )
+
 
 class TestFailureReportParsing(unittest.TestCase):
     """Tests for failure report field defaults and edge cases"""

tests/test_utils.py

@@ -249,6 +249,52 @@ class Test(unittest.TestCase):
             "Mon, 01 Jan 2024 00:00:00 +0000 (UTC)"
         )
         self.assertEqual(dt.year, 2024)
+        # A tab before the comment, and more than one comment, are stripped
+        # too.
+        self.assertEqual(
+            parsedmarc.utils.human_timestamp_to_datetime(
+                "Tue, 1 Jan 2019 00:00:00 +0000\t(GMT)"
+            ),
+            datetime(2019, 1, 1, tzinfo=timezone.utc),
+        )
+        self.assertEqual(
+            parsedmarc.utils.human_timestamp_to_datetime(
+                "Mon, 01 Jan 2024 00:00:00 +0000 (x) (y)"
+            ),
+            datetime(2024, 1, 1, tzinfo=timezone.utc),
+        )
+        # RFC 5322 section 3.2.2 lets comments nest and contain quoted-pairs;
+        # a regex cannot strip those, which is why a depth-counting scan does.
+        self.assertEqual(
+            parsedmarc.utils.human_timestamp_to_datetime(
+                "Mon, 01 Jan 2024 00:00:00 +0000 (a (b) c)"
+            ),
+            datetime(2024, 1, 1, tzinfo=timezone.utc),
+        )
+        self.assertEqual(
+            parsedmarc.utils.human_timestamp_to_datetime(
+                "Mon, 01 Jan 2024 00:00:00 +0000 (\\) x)"
+            ),
+            datetime(2024, 1, 1, tzinfo=timezone.utc),
+        )
+
+    def testHumanTimestampToDatetimeWhitespaceIsLinear(self):
+        """A whitespace-padded timestamp is rejected in linear time.
+
+        Comments were stripped with the regex ``\\s*\\(.*\\)\\s*``, whose
+        leading ``\\s*`` was retried from every position in a whitespace run
+        — quadratic, so 200,000 spaces took 18.6 s. The function runs on
+        every message's Date header. The 5 s bound is not flaky: the
+        depth-counting scan handles this input in milliseconds, orders of
+        magnitude below both the bound and the old pattern's cost. The
+        comment is left unclosed so the old pattern can never match: with a
+        closing parenthesis its first attempt succeeded and the backtracking
+        never happened."""
+        start = time.perf_counter()
+        # dateutil's ParserError subclasses ValueError.
+        with self.assertRaises(ValueError):
+            parsedmarc.utils.human_timestamp_to_datetime(" " * 200_000 + "(x")
+        self.assertLess(time.perf_counter() - start, 5)
 
     def testHumanTimestampToDatetimeNegativeZero(self):
         """-0000 timezone is handled"""