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

2 files changed, 308 insertions, 9 deletions

diff --git a/abusectl/report.py b/abusectl/report.py
index 7410ae2..7cb4a9b 100644
--- a/abusectl/report.py
+++ b/abusectl/report.py
@@ -44,7 +44,57 @@ _WIDTH = 72
 # resource that was never reported. Something must mark the seam, and this
 # marks it in the only direction that is safe: a fragment ANNOUNCES that it
 # is a fragment, rather than a whole value having to prove it is whole.
+#
+# Because the marker is a character a VALUE may also contain, every literal
+# backslash in a value is DOUBLED before wrapping and halved on the way
+# back. Without that, a value ending in "\" is indistinguishable from a wrap
+# marker, and the values here are attacker-supplied: a trailing backslash is
+# legal in a URL path, and an attacker reading this source could append one
+# to make their indicator garble itself in the report a desk reads.
+#
+# Doubling EVERY backslash rather than only a trailing one is what keeps the
+# encoding unambiguous. Escaping just the last character leaves "x\\" (two
+# literal backslashes) encoding to the same text as "x\" followed by a wrap,
+# which is the same bug one character further along.
 _CONTINUATION = "\\"
+_ESCAPE = "\\"
+
+
+def _escape(value: str) -> str:
+    """Double every backslash so none can be read as a continuation marker.
+
+    The inverse is _unescape(). Applied to the value only, never to the
+    indent or the surrounding prose, so what a reader sees differs from the
+    literal value in exactly one way and unwrap() undoes exactly that.
+    """
+    return value.replace(_ESCAPE, _ESCAPE + _ESCAPE)
+
+
+def _unescape(value: str) -> str:
+    """Halve the doubled backslashes _escape() produced.
+
+    Scans left to right, consuming a doubled pair as one character. On text
+    _escape() actually produced, every run is even and str.replace gives
+    the same answer, which a mutation test confirmed over every backslash
+    pattern up to length 11: this is NOT protecting the round trip, and
+    saying otherwise would overstate what the tests hold down.
+
+    It is kept because unwrap() is public and may be handed a line a user
+    edited, where a run can be odd. The scan then consumes pairs strictly
+    left to right and leaves the odd one alone, which is the reading that
+    matches how the text was written; str.replace rescans its own output
+    and would fold a stray backslash into the pair beside it.
+    """
+    out = []
+    index = 0
+    while index < len(value):
+        if value.startswith(_ESCAPE + _ESCAPE, index):
+            out.append(_ESCAPE)
+            index += 2
+        else:
+            out.append(value[index])
+            index += 1
+    return "".join(out)
 
 
 def _wrap_value(value: str, indent: str) -> list[str]:
@@ -59,16 +109,44 @@ def _wrap_value(value: str, indent: str) -> list[str]:
     An arbitrary break plus an explicit marker is legible precisely because
     the marker, not the position, carries the meaning.
 
-    The value is never altered, only divided; unwrap() is the exact inverse
-    and there is a test asserting the round trip on a 120-character URL.
+    The value is ESCAPED first, so its own backslashes cannot be mistaken
+    for the marker, and the wrap arithmetic then runs over the escaped text:
+    the column limit governs what is PRINTED, and the escaped form is what
+    is printed. Measuring the original instead would let a value full of
+    backslashes overflow the line.
+
+    A break must NEVER land between the two halves of an escaped pair, and
+    the loop below backs off by one character to guarantee it. This is not
+    tidiness: unwrap() decides whether a trailing backslash is a marker or
+    content by the PARITY of the run it ends, and a break inside a pair
+    splits that run across two lines, so both halves are counted wrongly.
+    A genuine marker then reads as content, the continuation line is
+    orphaned, and the tail of the value is silently dropped.
+
+    That defect survived a first fix and a passing test suite, because it
+    needs a backslash to land exactly on the break column: it appears in
+    random adversarial values roughly one time in eight and in none of the
+    hand-written cases. Backing off one character costs a column on a line
+    that has a backslash at its edge, and buys an invariant that holds for
+    every input rather than for the inputs someone thought of.
+
+    The value is never altered, only divided and escaped; unwrap() is the
+    exact inverse, asserted over adversarial values including trailing and
+    doubled backslashes.
     """
+    value = _escape(value)
     room = _WIDTH - len(indent) - len(_CONTINUATION)
     if len(indent) + len(value) <= _WIDTH:
         return [indent + value]
     lines = []
     while len(value) > room:
-        lines.append(indent + value[:room] + _CONTINUATION)
-        value = value[room:]
+        cut = room
+        # An odd run of backslashes ending at the cut means the last one is
+        # the first half of a pair; move the break before it.
+        if (len(value[:cut]) - len(value[:cut].rstrip(_ESCAPE))) % 2 == 1:
+            cut -= 1
+        lines.append(indent + value[:cut] + _CONTINUATION)
+        value = value[cut:]
     lines.append(indent + value)
     return lines
 
@@ -87,14 +165,36 @@ def unwrap(text: str) -> str:
     content: no value this module emits begins with a space, because every
     one of them is an indicator, a header value or an identity, all of
     which are stripped before they arrive.
+
+    A trailing backslash is a marker only when the run of backslashes it
+    ends is ODD, because _escape() doubled every literal one. An even run is
+    entirely escaped content and the line ends there. Testing endswith("\\")
+    alone was the defect this parity check replaces: it read a value's own
+    trailing backslash as a marker and swallowed the following line, which
+    for a Subject meant absorbing the Date header beneath it.
+
+    Rejoining happens BEFORE unescaping, so a doubled pair split across a
+    break is whole again before it is decoded.
     """
-    out = []
+    joined = []
     for line in text.splitlines():
-        if out and out[-1].endswith(_CONTINUATION):
-            out[-1] = out[-1][:-len(_CONTINUATION)] + line.lstrip()
+        if joined and _ends_with_marker(joined[-1]):
+            joined[-1] = joined[-1][:-len(_CONTINUATION)] + line.lstrip()
         else:
-            out.append(line)
-    return "\n".join(out)
+            joined.append(line)
+    return "\n".join(_unescape(line) for line in joined)
+
+
+def _ends_with_marker(line: str) -> bool:
+    """Whether this line ends in a continuation marker rather than content.
+
+    The marker is one unescaped backslash, and _escape() doubled every
+    literal one, so the question is purely the PARITY of the trailing run:
+    odd means the last backslash has no partner and is the marker, even
+    means every one is half of an escaped pair and the line ends here.
+    """
+    run = len(line) - len(line.rstrip(_ESCAPE))
+    return run % 2 == 1
 
 
 # What each parse.py origin means in a sentence a desk can act on.
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()