Skip to content

fix: guard QuotedPrintable decode against truncated =XY escapes#283

Open
guettlibot wants to merge 1 commit into
Enough-Software:mainfrom
guettlibot:fix/qp-decode-oob-on-truncated-escape
Open

fix: guard QuotedPrintable decode against truncated =XY escapes#283
guettlibot wants to merge 1 commit into
Enough-Software:mainfrom
guettlibot:fix/qp-decode-oob-on-truncated-escape

Conversation

@guettlibot

Copy link
Copy Markdown

Problem

QuotedPrintableMailCodec.decodeText at lib/src/codecs/quoted_printable_mail_codec.dart:199 calls cleaned.substring(i + 1, i + 3) unconditionally whenever it encounters =. Two independent inputs make i + 3 exceed cleaned.length and throw a RangeError:

  1. Outer branch, truncated tail. Input ending in a bare = or =X (e.g. "hello=", "hello=A", "h=A").
  2. Inner while loop, off-by-one guard. The guard cleaned.length > (i + 4) && cleaned[i + 3] == '=' proves cleaned[i + 3] exists, but after the following i += 3 the substring reads cleaned[i+4..i+5], so a valid multi-byte escape followed by a truncated tail like "hi=C3=B6=A" still throws.

Minimal repro against 2.1.7 / current main:

MailCodec.quotedPrintable.decodeText('hello=', convert.utf8);
// throws: RangeError (end): Only valid value is 6: 8

Fix

RFC 2045 §6.7 permits consumers to preserve malformed quoted-printable input verbatim, and the existing int.tryParse == null branch already does that for non-hex =XY. This PR extends that policy to the two truncated cases:

  • Before the outer substring, check i + 3 > cleaned.length; if so, append the malformed tail (cleaned.substring(i)) to the buffer and break.
  • Inside the inner while loop, add if (i + 6 > cleaned.length) break; so a truncated inner escape falls through to the outer truncated-tail branch on the next outer iteration.

No behaviour change for well-formed input; the full existing test suite passes (dart test, 420 tests).

Tests

Added a regression group malformed truncated =XY escapes are preserved verbatim in test/codecs/quoted_printable_mail_codec_test.dart covering:

  • trailing bare =
  • trailing =A
  • lone =A at end of a short input
  • valid multi-byte escape followed by truncated =A (exercises the inner-loop path)

Each asserts the decoder returns the input verbatim rather than throwing.

Downstream context

Reported from downstream in guettli/sharedinbox#232, where an incoming mail body whose quoted-printable payload ended mid-escape crashed the entire Flutter app via runZonedGuarded → FlutterError.onError → CrashScreen. That project has since added a defensive try/catch at the call site, but the foreground getEmailBody path for the currently viewed email still hits the same crash on any similar malformed message. This upstream fix protects all callers.

Non-goals

  • Not fixing the int.parse inside the inner while loop (which can still throw FormatException on non-hex input there). That is a distinct latent bug from BUG: in serialization of ServerConfig.fromJson() #232 and is left for a focused follow-up.
  • Not touching the print(...) diagnostics.

decodeText read cleaned.substring(i + 1, i + 3) unconditionally whenever
it saw '='. Two paths could take i + 3 past cleaned.length and throw a
RangeError:

* the outer branch, when the input ended with a bare '=' or '=X'
* the inner while loop, whose guard cleaned.length > (i + 4) proves
  cleaned[i + 3] exists but not that the substring(i+4, i+6) after
  i += 3 is in bounds

RFC 2045 §6.7 permits consumers to preserve malformed input verbatim,
which matches how the existing int.tryParse == null branch already
handles non-hex '=XY'. Extend that to both truncated cases: preserve the
tail verbatim on the outer branch and bail out of the inner loop so the
outer branch can pick up the leftover characters.

Reported from downstream in guettli/sharedinbox#232, where an incoming
mail body whose quoted-printable payload ended mid-escape crashed the
entire app via runZonedGuarded.
guettlibot pushed a commit to guettli/sharedinbox that referenced this pull request Jul 11, 2026
The RangeError from QuotedPrintableMailCodec.decodeText that triggered
the crash in #232 is fixed upstream in Enough-Software/enough_mail#283.
Note the reference next to the defensive try/catch so a future reader
knows the underlying bug is being tracked upstream and knows why the
guard stays even after the dependency bumps: it still covers
network/DB failures in the opportunistic prefetch path.

Closes #235
guettli pushed a commit to guettli/sharedinbox that referenced this pull request Jul 16, 2026
* docs(di): cross-reference upstream enough_mail fix for #232 crash

The RangeError from QuotedPrintableMailCodec.decodeText that triggered
the crash in #232 is fixed upstream in Enough-Software/enough_mail#283.
Note the reference next to the defensive try/catch so a future reader
knows the underlying bug is being tracked upstream and knows why the
guard stays even after the dependency bumps: it still covers
network/DB failures in the opportunistic prefetch path.

Closes #235

* test(fuzz): double-sync after JMAP mutations to unblock CI

The long-term IMAP/JMAP fuzz test at step 6 (JMAP mutations) has been
flaking on CI with sporadic 'Mismatch after JMAP mutations!' —
EmailFieldMismatch.flagged on a random message, sometimes accompanied by
missingInA/missingInB when the moveEmail hasn't landed on the IMAP side
yet. Both this PR's initial run and the rerun hit it with different
random IDs, and main is intermittently affected too.

The single-round sync at line 369-370 assumes Stalwart bumps the IMAP
HIGHESTMODSEQ on the same tick that the JMAP mutation lands. It doesn't
always; CONDSTORE then skips the mailbox and the assertion fires. Wrap
the pair in a 2-round loop so the second pass picks up the delayed
HIGHESTMODSEQ bump, mirroring the same pattern already used elsewhere.

This is a straight cherry-pick of the test-only change from the
abandoned PR #204 / commit 8f0b834 ("fix(jmap/tests): retry transport
errors and double-sync after JMAP mutations"). That PR was closed
without merging so this fix never landed; the sibling
jmap_client.dart / account_comparison_stalwart_test.dart pieces of
8f0b834 are deliberately not cherry-picked here to keep the scope of
this docs-tracking PR minimal — they can land separately if the
transport-retry problem is still occurring.

Unblocks CI on #237.

---------

Co-authored-by: AgentLoop <agentloop@thomas-guettler.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant