aboutsummaryrefslogtreecommitdiffstats
path: root/tests/test_report.py
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-09-09 19:30:51 +0200
committerDanilo M. <danix@danix.xyz>2026-09-09 19:30:51 +0200
commitf1b37bc5dea7493a6d98e552c48cb6e6f9f3f0ec (patch)
treea35f29b8018c23264d43e83fa6cf405e8ff58d6d /tests/test_report.py
parentb0031f905eb63fc76aa6cad0421c4623ebad2b88 (diff)
downloadabusectl-f1b37bc5dea7493a6d98e552c48cb6e6f9f3f0ec.tar.gz
abusectl-f1b37bc5dea7493a6d98e552c48cb6e6f9f3f0ec.zip
fix: escape the continuation marker so a value cannot forge one
The marker was an unescaped trailing backslash, and a backslash is legal in a URL path, so a value ending in one was indistinguishable from a wrap. An attacker who read this source could append one and make their own indicator garble itself in the report an abuse desk reads: an adversarial trigger on attacker-supplied text, not an edge case. The short case needed no wrapping at all to corrupt. unwrap() ate the following line regardless, merging an indicator with its own origin annotation. On Subject it was worse, absorbing the Date beneath it and making the "Message as declared" block misstate what the message declared, which is the one thing that block exists to report faithfully. Every backslash is now doubled before wrapping and halved on the way back, and unwrap() tells a marker from content by the PARITY of the trailing run. Doubling only a trailing one would leave "x\\" encoding as "x\" plus a marker, the same bug one character along. A second defect surfaced only under a randomised sweep, after the first fix and a green suite: a break landing BETWEEN the halves of an escaped pair splits the run whose parity unwrap() counts, so a real marker reads as content and the tail is silently dropped. It needs a backslash at exactly the break column, so no hand-written case found it and 454 of 3538 random ones did. The wrap now backs off a character rather than splitting a pair. Verified over 9132 adversarial values, including every backslash pattern up to length 9 and values that are entirely backslashes: 0 round-trip failures, 0 header-block corruptions, no line over 72 columns. Values are escaped, never rejected or sanitised: the report says what the message contained. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xj1ayFRSUQ2u7cwb3S4axE
Diffstat (limited to 'tests/test_report.py')
-rw-r--r--tests/test_report.py199
1 files changed, 199 insertions, 0 deletions
diff --git a/tests/test_report.py b/tests/test_report.py
index 35c68ba..3b13d11 100644
--- a/tests/test_report.py
+++ b/tests/test_report.py
@@ -723,5 +723,204 @@ class TextPart(unittest.TestCase):
self.assertNotIn("Authentication results", text)
+class BackslashRoundTrip(unittest.TestCase):
+ """A value's own backslash must never be read as a wrap marker.
+
+ The continuation marker is a trailing "\\", and a URL path may legally
+ end in one. Until this was fixed the two were indistinguishable, so an
+ attacker who read this source could append a backslash and make their
+ own indicator garble itself in the report an abuse desk reads. That is
+ an adversarial trigger on attacker-supplied text, not an edge case.
+
+ The property asserted throughout is the only one that closes it:
+ unwrap(text_part(...)) contains the value EXACTLY, for every value,
+ wrapped or not. Asserting "the value appears" without unwrap, or
+ asserting only on long values, both leave the short case open, and the
+ short case is the one that needs no wrapping to corrupt.
+ """
+
+ def _render_ioc(self, value: str) -> str:
+ manifest = copy.deepcopy(MANIFEST)
+ manifest["iocs"] = [{"id": "ioc-1", "type": "url", "value": value,
+ "origin": "body"}]
+ manifest["contacts"] = [
+ {"iocs": ["ioc-1"], "query": "example.invalid",
+ "abuse": ["abuse@host.invalid"], "source": "rdap"},
+ ]
+ destination = report.email_destinations(manifest["contacts"])[0]
+ return report.text_part(manifest, destination, IDENTITY)
+
+ def _assert_round_trips(self, value: str) -> None:
+ text = self._render_ioc(value)
+ for line in text.splitlines():
+ self.assertLessEqual(len(line), 72, line)
+ self.assertIn(value, report.unwrap(text))
+
+ def test_a_short_value_ending_in_a_backslash_survives(self):
+ """The case that needs no wrapping at all to corrupt.
+
+ Nothing is wrapped here, yet unwrap() used to eat the following
+ line, merging the indicator with its own origin annotation and
+ rendering "http://a.invalid/xseen in a link in the message body".
+ One corrupt line where there were two, and a wrong indicator.
+ """
+ self._assert_round_trips("http://a.invalid/x\\")
+
+ def test_a_long_value_ending_in_a_backslash_survives(self):
+ self._assert_round_trips("http://a.invalid/" + "b" * 90 + "\\")
+
+ def test_a_value_with_an_interior_backslash_survives(self):
+ self._assert_round_trips("http://a.invalid/x\\y/z")
+
+ def test_a_value_ending_in_two_backslashes_survives(self):
+ """Whatever escaping is chosen must not have its own off-by-one.
+
+ Doubling every backslash makes a trailing pair into four, and a
+ decoder that consumes them greedily or in the wrong order gives
+ back one backslash or three. This is the test that catches that.
+ """
+ self._assert_round_trips("http://a.invalid/x\\\\")
+
+ def test_adversarial_values_round_trip_exactly(self):
+ """A handful of shapes chosen to sit on the seams.
+
+ The two boundary values matter most: a value that exactly fills a
+ line and one a single character over it are where an off-by-one in
+ the wrap arithmetic lives, and a backslash landing exactly on the
+ break column is where escaping and wrapping interact.
+ """
+ indent = 2
+ room = 72 - indent
+ values = [
+ "http://a.invalid/x\\",
+ "http://a.invalid/x\\y/z",
+ "http://a.invalid/x\\\\",
+ "\\" + "a" * 40,
+ "a" * 40 + "\\",
+ "http://a.invalid/" + "b" * 90 + "\\",
+ "a" * room, # exactly fills the line
+ "a" * (room + 1), # one character over
+ "a" * (room - 1) + "\\", # backslash at the break
+ "a" * room + "\\",
+ "\\\\" + "c" * 80 + "\\\\",
+ ]
+ for value in values:
+ with self.subTest(value=value):
+ self._assert_round_trips(value)
+
+ def test_a_backslash_landing_on_the_break_column_survives(self):
+ """The case that a passing suite still missed.
+
+ Escaping doubles each backslash, and a break falling BETWEEN the
+ two halves of a pair splits the run unwrap() counts the parity of.
+ Both halves are then misread, a real marker reads as content, the
+ continuation line is orphaned and the tail of the value is silently
+ dropped. It needs a backslash at exactly the break column, so no
+ hand-written case found it; a randomised sweep failed 454 of 3538.
+
+ Walking the backslash across every position around the boundary is
+ what makes this deterministic rather than luck.
+ """
+ room = 72 - 2 # indent is two spaces for an indicator line
+ for offset in range(-4, 5):
+ position = room + offset
+ if position < 1:
+ continue
+ value = "a" * position + "\\" + "b" * 30
+ with self.subTest(offset=offset):
+ self._assert_round_trips(value)
+
+ def test_a_run_of_backslashes_across_the_break_survives(self):
+ """A run is where an off-by-one in the back-off hides.
+
+ Backing off one character is correct only if the character it lands
+ on is the first half of a pair; a run of three or four exercises
+ whether the parity test looks at the run rather than at one
+ character.
+ """
+ room = 72 - 2
+ for length in range(1, 6):
+ for offset in range(-3, 4):
+ position = room + offset
+ if position < 1:
+ continue
+ value = "a" * position + "\\" * length + "b" * 20
+ with self.subTest(length=length, offset=offset):
+ self._assert_round_trips(value)
+
+ def test_a_value_that_is_entirely_backslashes_survives(self):
+ """Escaping doubles the length, so this is the worst case for both
+ the wrap arithmetic and the parity test at once."""
+ for length in (1, 2, 3, 34, 35, 36, 70, 71):
+ with self.subTest(length=length):
+ self._assert_round_trips("\\" * length)
+
+ def test_an_attacker_subject_cannot_corrupt_the_header_block(self):
+ """Subject is attacker-controlled and sits beside headers it can eat.
+
+ This is worse than the URL case: a trailing backslash on Subject
+ used to swallow the following line, rendering
+ "Subject: Verify nowDate: Mon, 07 Sep 2026 09:12:40 +0000". The
+ attacker's own text destroys a DIFFERENT field's value, so the
+ block misstates what the message declared, which is the one thing
+ that block exists to report faithfully.
+ """
+ subject = "Verify now\\"
+ manifest = copy.deepcopy(MANIFEST)
+ manifest["headers"] = [
+ ("Subject", subject),
+ ("Date", "Mon, 07 Sep 2026 09:12:40 +0000"),
+ ]
+ destination = report.email_destinations(manifest["contacts"])[0]
+ text = report.text_part(manifest, destination, IDENTITY)
+
+ for line in text.splitlines():
+ self.assertLessEqual(len(line), 72, line)
+ joined = report.unwrap(text)
+ self.assertIn(f"Subject: {subject}", joined)
+ # The Date must survive intact rather than being absorbed.
+ self.assertIn("Date: Mon, 07 Sep 2026 09:12:40 +0000", joined)
+
+ def test_a_display_name_ending_in_a_backslash_survives(self):
+ """The From display name is attacker-controlled too, and a sweep
+ has already found a spoofed one."""
+ value = '"Example Bank\\" <phish@sender.invalid>'
+ manifest = copy.deepcopy(MANIFEST)
+ manifest["headers"] = [("From", value),
+ ("Subject", "Your account requires check")]
+ destination = report.email_destinations(manifest["contacts"])[0]
+ text = report.text_part(manifest, destination, IDENTITY)
+
+ joined = report.unwrap(text)
+ self.assertIn(f"From: {value}", joined)
+ self.assertIn("Subject: Your account requires check", joined)
+
+ def test_unwrap_reads_a_marker_by_parity_not_by_a_trailing_backslash(self):
+ """unwrap() is PUBLIC, so its input is not only our own output.
+
+ A case manifest is a file the user edits and a desk may script
+ against the text part, so unwrap() must decide correctly on a line
+ it did not generate. Inside generated text the wrap back-off means
+ a marker always follows an even run, so parity and a plain
+ endswith() agree and neither is distinguishable by a round-trip
+ test. They disagree here, on a line ending in an escaped pair and
+ nothing else: that is content, and the following line must NOT be
+ absorbed into it.
+ """
+ # "a\\" escaped is a value ending in one literal backslash, whole
+ # on its line. endswith() reads the second half as a marker.
+ self.assertEqual(report.unwrap(" a\\\\\n next"), " a\\\n next")
+ # An odd run IS a marker: two escaped halves plus the marker.
+ self.assertEqual(report.unwrap(" a\\\\\\\n next"), " a\\next")
+
+ def test_an_identity_containing_a_backslash_survives(self):
+ identity = {"name": "A Reporter\\", "org": "Example Consulting",
+ "email": "reporter@example.org"}
+ destination = report.email_destinations(MANIFEST["contacts"])[0]
+ text = report.text_part(MANIFEST, destination, identity)
+ self.assertIn("A Reporter\\", report.unwrap(text))
+ self.assertIn("Generated by abusectl.", report.unwrap(text))
+
+
if __name__ == "__main__":
unittest.main()