diff --git a/src/jmap-email/Dockerfile b/src/jmap-email/Dockerfile index 6a8e6421..02a7f3b9 100644 --- a/src/jmap-email/Dockerfile +++ b/src/jmap-email/Dockerfile @@ -2,7 +2,7 @@ # (managed CPython 3.14.6 + uv). NOTE: the PyPI *release* still builds on the # official python:3.14.6-slim image via bin/release-jmap-email.sh — that is the # release-parity environment; this image is only for CI lint/type/test. -# Zero non-stdlib runtime deps; only pytest + hypothesis for the suite, plus +# One runtime dep (idna); plus pytest + hypothesis for the suite, and # ``ty``/``ruff``/``pylint`` for type-checking and linting. ARG PYTHON_UV_IMAGE=messages-python-uv:local FROM ${PYTHON_UV_IMAGE} @@ -29,7 +29,7 @@ COPY pyproject.toml README.md LICENSE CHANGELOG.md ./ COPY jmap_email ./jmap_email COPY tests ./tests -# Install the package itself (no runtime deps; zero new packages resolved). +# Install the package itself (resolves its one runtime dep, idna). # Editable so that the mount-overlay in development instantly picks up edits. RUN uv pip install --no-cache -e . diff --git a/src/jmap-email/README.md b/src/jmap-email/README.md index 4063e861..77782f5f 100644 --- a/src/jmap-email/README.md +++ b/src/jmap-email/README.md @@ -2,9 +2,12 @@ A strict-JMAP RFC 8621 Email object library for Python 3.14+, with lenient RFC 5322 / MIME parsing and strict-by-design composition. -**Zero runtime dependencies** — the package is a clean wrapper around +**One runtime dependency** — the package is a clean wrapper around the Python stdlib `email` package, plus null-safe shape accessors over -the JMAP Email object. +the JMAP Email object. The single dependency is +[`idna`](https://pypi.org/project/idna/), for UTS 46 non-transitional +domain encoding: the stdlib codec is IDNA2003, which silently folds +`faß.de` into the *distinct* registrable domain `fass.de`. The codebase came out of operating an inbound mail pipeline; every CVE and research result in the [defense matrix](#defense-matrix) below has @@ -109,11 +112,67 @@ These fields are NOT in RFC 8621 — they expose information the parser already computes so consumers don't have to re-walk the message: - `_ext.defects` — stdlib `MessageDefect` class names collected during - the parse walk; useful for message-store quarantine policies (the - Mailman pattern). + the parse walk, plus the parser-ambiguity markers below; useful for + message-store quarantine policies (the Mailman pattern). - `_ext.resent` — Resent-* typed projection (see below). Present only when the wire carries at least one Resent-* header. +#### Parser-ambiguity markers + +A single email is parsed several times on its way to a reader — by the +receiving server, by a spam filter, by a virus scanner, finally by the +client that displays it. Some MIME constructs are resolved differently +by each, and that gap is exploitable: a filter can be made to read past +content the client goes on to render. Differential fuzzing of Postfix, +SpamAssassin, ClamAV, Evolution and Thunderbird demonstrates working +smuggling on exactly these constructs.[^mime2025] + +This parser resolves each the way the stdlib does and carries on. What +it will not do is hand you a confident parse without telling you a +choice was made, so each construct also lands in `_ext.defects`: + +| Marker | Construct | Why it matters | +| --- | --- | --- | +| `DuplicateFromDefect` | more than one `From` | one identity for the filter that authenticated the message, another for the human who reads it (CERT VU#517845) | +| `DuplicateScalarHeaderDefect` | any other RFC 5322 §3.6 max=1 header twice | `Subject`, `Date`, `Message-ID`, … — first wins here, last may win elsewhere | +| `DuplicateContentTypeDefect` | more than one `Content-Type` | we take the first; a client honouring the second sees a multipart tree, and its attachments never reach you | +| `DuplicateTransferEncodingDefect` | more than one `Content-Transfer-Encoding` | filter and client can disagree on whether the body is encoded at all | +| `UnrecognizedTransferEncodingDefect` | token outside RFC 2045 §6.1 (`bas64`, `: base64`, …) | we leave the body undecoded; lenient clients guess and reveal it | +| `DuplicateBoundaryParameterDefect` | two `boundary=` parameters | whichever we honour, the other delimits parts we never see | +| `MissingMimeVersionDefect` | MIME syntax with no `MIME-Version` | a strict receiver reads the body as flat text and never sees the parts | +| `NonEmptyPreambleDefect` | text before the first boundary | RFC 2046 §5.1 says ignore it, and we do — Thunderbird and Evolution show it as the first line | +| `NonEmptyEpilogueDefect` | text after the closing boundary | the same gap at the other end of the body | +| `ConflictingAttachmentNameDefect` | part names itself twice, differently | `Content-Type: name=` vs `Content-Disposition: filename=` vs `filename*` — the recipient saves it under a name you never saw | +| `ControlCharInHeaderDefect` | NUL or control character in a MIME header | read three ways: stripped (us), truncated at it, or kept — one part, three filenames | +| `EmptyBoundaryDefect` | multipart with an absent or empty `boundary=` | no agreed way to split the body | +| `EncodedWordInParameterDefect` | RFC 2047 `=?…?=` in a MIME parameter | not permitted there (RFC 2231 is the mechanism); we decode it, others show it raw | +| `FoldInQuotedParameterDefect` | a header folds *inside* a quoted parameter value | `filename="pay load.exe"` — we unfold to `pay load.exe`, a parser dropping the whole fold reads `payload.exe`, one truncating at CR reads `pay` | +| `PartialMessageDefect` | `message/partial` | RFC 2046 §5.2.2 splits one message across several; the payload exists only after a reassembly a per-message scanner never does | +| `ExternalBodyDefect` | `message/external-body` | the content is fetched from elsewhere, so it is not in this message for anyone to scan | +| `AddressListTruncatedDefect` | address header past `max_address_list_bytes` | entries past the cut are dropped, so the list we report is shorter than the wire's — and empty (`[]`) when no mailbox separator precedes the cap. A recipient you never see is one you cannot act on | + +None is an error, and well-formed mail raises none of them. Treat them +as input to a policy: score them, quarantine on them, or log them and +move on. + +One related scope boundary, which is *not* a marker because it would +fire on every forwarded message: attachments nested inside a +`message/rfc822` part are not enumerated in the outer `attachments` +list. The nested message arrives whole, as one attachment carrying its +raw bytes in `content` — re-run `parse_email` on those bytes if you +scan attachments, or a payload one level down stays invisible to you. + +The last three are the anomaly classes named by +[draft-chen-email-mime-ambiguity-defense][ietf-mime] that the stdlib does +not already flag for you. + +[^mime2025]: S. B. Andarzian, M. Meyers, E. Poll, *Email Smuggling with +Differential Fuzzing of MIME Parsers*, Radboud University, 2025. See also +J. Chen et al., *Inbox Invasion: Exploiting MIME Ambiguities to Evade +Email Attachment Detectors*, CCS '24. + +[ietf-mime]: https://datatracker.ietf.org/doc/draft-chen-email-mime-ambiguity-defense/ + ### `EmailBodyPart` extensions RFC 8621 §4.1.4 lists the `EmailBodyPart` shape as `partId`, `blobId`, @@ -177,9 +236,12 @@ the contract is explicit: spec-faithful value would crash any downstream insert. Carrying them through and dropping them at the storage boundary would also be wrong (different stores would handle them differently). - - Truncate at `max_header_value_bytes` (default 102 400) — the stdlib - `_header_value_parser` has quadratic-time hot spots on adversarial - inputs (gh-136063); truncating early bounds wall-clock. + - Refuse a field above `max_header_value_bytes` (default 102 400): + `parse_email` returns `None` rather than truncating — there is no + generally safe place to cut an arbitrary field (see the options + table below). Bounding the input also sidesteps the stdlib + `_header_value_parser`'s quadratic-time hot spots on adversarial + inputs (gh-136063). The `EmailBodyPart.headers[i].value` field follows the same policy. - **Inline media isn't added to `attachments` in the `multipart/alternative` @@ -207,7 +269,21 @@ leaves to the server, such as the `preview` length. | `max_preview_chars` | 256 | RFC 8621 §4.1.4 `preview` ceiling | | `max_preview_scan_bytes` | 131 072 | bound `preview` work on markup-only input | -Excess input is silently truncated and logged at WARNING level. +Most excess input is silently truncated and logged at WARNING level. + +`max_header_value_bytes` is the exception: a field above it makes +`parse_email` return `None`. RFC 5322 §2.2.3 puts no limit on a header +field — "an unfolded header field has no length restriction and +therefore may be indeterminately long" — so this is local policy, set to +Postfix's `header_size_limit`. Postfix discards the excess; we refuse the +message, because there is no generally safe place to cut an arbitrary +field. A shortened `Received` still looks well-formed to trust-scope +logic, and cutting an address list mid-token *manufactures* an address +nobody sent. `max_address_list_bytes` truncation, which stays below that +ceiling, therefore cuts back to a top-level mailbox separator — the same +shape as Postfix's `header_address_token_limit`, which discards excess +*tokens* rather than bytes — and records +`AddressListTruncatedDefect`. A single process can host multiple workloads with different options — they travel with the call, never via shared module state: @@ -226,6 +302,107 @@ parse_email(inbound_smtp_bytes, options=gateway) across threads and as cache keys. (Pre-0.2 this was `ParseLimits`, passed as `limits=`; both were renamed outright — see the [CHANGELOG](CHANGELOG.md).) +`DEFAULT_PARSE_OPTIONS` is the instance used when you pass no `options=`. +Every field has a default, so you rarely need it to *build* options — +`ParseOptions(max_mime_parts=500)` already inherits the rest. It's there to +read the shipped values without constructing anything: + +```python +from jmap_email import DEFAULT_PARSE_OPTIONS + +budget = DEFAULT_PARSE_OPTIONS.max_preview_chars # 256 +``` + +## Compose options + +`ComposeOptions` is the compose-side peer of `ParseOptions` — same frozen, +hashable, `dataclasses.replace`-able shape, passed the same way: + +```python +from jmap_email import ComposeOptions, compose_email + +raw = compose_email(jmap, options=ComposeOptions(idna_encode_domains=True)) +``` + +Where `ParseOptions` is mostly resource caps against hostile input, these +are output-correctness choices — each names a place where "strict" is +deployment-dependent rather than universal. + +| Field | Default | Effect | +| --- | --- | --- | +| `emit_bcc` | `False` | Emit the `Bcc:` header. The entire point of Bcc is that it must not be transmitted; set `True` only for archive reconstruction (PST import), where the list was already in the source. | +| `idna_encode_domains` | `False` | IDNA-encode a non-ASCII **domain** to its A-label form (`contact@exemplé.fr` → `contact@xn--exempl-gva.fr`). | +| `allow_8bit` | `False` | Emit non-ASCII bodies as raw 8-bit instead of base64/QP. Requires 8BITMIME (RFC 6152) on the hop. | +| `allow_smtputf8` | `False` | Emit UTF-8 headers (RFC 6532) and permit a non-ASCII **local part**. Requires SMTPUTF8 (RFC 6531) on the hop. Implies `allow_8bit`. | + +The last two name ESMTP capabilities of the hop the bytes are headed +for. This library does not discover those, and does not assume them — +hence the conservative defaults, which produce pure-ASCII 7-bit output +that any MTA accepts. How you learn them is yours: a relay whose +configuration you own, an EHLO response you read *before* composing, or +two variants composed up front so the delivery loop can fall back. + +`in_reply_to` and `prepend_headers` stay keyword arguments on +`compose_email`: they are per-*message* data that changes on every call, +where everything in `ComposeOptions` is a property of the call site. + +### Non-ASCII addresses + +`idna_encode_domains` governs the domain, and deliberately only the domain. +Punycode is a DNS algorithm (RFC 3492/5891) and a local part is not a DNS +label, so an IDN domain has an exact ASCII wire form — the same one the MX +lookup has to use — and a non-ASCII local part has none. + +A non-ASCII **local part** therefore needs `allow_smtputf8`, not this +flag: with `allow_smtputf8=True` the whole addr-spec travels as UTF-8, and +without it the address raises `InvalidAddressError`. Carrying one requires +SMTPUTF8 (RFC 6531), which is negotiated per-hop against the receiver's +EHLO, is viral across every address in the transaction, and has **no +downgrade path** — RFC 6530 dropped the mechanism RFC 5504 had specified. +Support is also not transitive: a relay that accepts your transaction may +itself have to forward to a hop that doesn't. That is why the ASCII +fallback variant below still rejects such an address: there is no ASCII +form of it to fall back *to*. + +Left at the default, a non-ASCII *domain* raises too, because the composer +does not rewrite an address you handed it unless you ask. + +### Composing a fallback pair + +Because SMTPUTF8 is per-hop and cannot be discovered ahead of time, the +usual pattern is to compose both forms and let the delivery loop choose: + +```python +from jmap_email import ComposeOptions, ComposeError, compose_email + +preferred = compose_email(jmap, options=ComposeOptions(allow_smtputf8=True)) +try: + fallback = compose_email(jmap, options=ComposeOptions(idna_encode_domains=True)) +except ComposeError: + fallback = None # no ASCII form exists — see below + +smtp.ehlo() +if smtp.has_extn("smtputf8"): + smtp.sendmail(sender, rcpts, preferred, mail_options=["SMTPUTF8", "BODY=8BITMIME"]) +elif fallback is not None: + smtp.sendmail(sender, rcpts, fallback) +else: + ... # bounce: RFC 6530 provides no downgrade for a UTF-8 mailbox name +``` + +An ASCII fallback exists whenever only the *domain* is non-ASCII. When +the **local part** is non-ASCII there is none — RFC 6530 dropped the +downgrade mechanism RFC 5504 had specified — and `compose_email` raising +is how you find that out. Bouncing is the specified behaviour, not a +gap in this library. + +Worth knowing what the standard library does here, since it is not this: +under `email.policy.default` it RFC 2047-encodes a non-ASCII domain, +emitting `contact@=?utf-8?q?exempl=C3=A9?=.fr`, which RFC 2047 §5 forbids +inside an addr-spec and no MTA routes. It never punycodes, at any layer — +`smtplib.send_message` buckets any non-ASCII address straight to SMTPUTF8 +or raises `SMTPNotSupportedError`. Converting the domain is left to you. + ## Strict-compose, lenient-parse The two entry points use **different stdlib `email.policy` instances @@ -313,6 +490,34 @@ About `now_sent_at()`: returns the current UTC time formatted as the ISO-8601 string `compose_email` expects for `sentAt`. One-liner instead of `datetime.now(timezone.utc).isoformat()`. +## Reading raw headers + +`parsed["headers"]` is deliberately RFC 8621 **Raw** form — byte-faithful, +*not* encoded-word-decoded (see [Conformance](#conformance)). That keeps the +header list honest for anything that needs the wire bytes, and it means any +header you read yourself may still be RFC 2047 encoded, or be a date string +nobody has parsed. Two helpers close that gap: + +```python +from jmap_email import decode_rfc2047_header, parse_date + +raw = find_header(parsed, "X-Gmail-Labels") # "=?UTF-8?Q?Re=C3=A7us?=" +label = decode_rfc2047_header(raw) # "Reçus" + +when = parse_date("Mon, 8 Jun 2026 14:30:00 +0200") # datetime | None +``` + +`decode_rfc2047_header` handles the mixed-charset, multi-word case and +returns a plain `str`. It is hardened against the encoded-word attacks in +the [defense matrix](#defense-matrix) — gh-114906 embedded newlines and +Mailsploit NUL truncation — which is the reason to use it over calling +`email.header.decode_header` yourself. + +`parse_date` returns `None` instead of raising on the malformed dates real +archives are full of, where the stdlib `parsedate_to_datetime` throws. Use +it for any date header you read raw; the `sentAt` field is already parsed +for you, and `sent_at_to_datetime` converts *that* back to a `datetime`. + ## Preview extraction `preview_text` turns an HTML or html2text-style body into the single, @@ -367,6 +572,47 @@ history is dropped via `>` lines and `
` only. Locale/product heuristics — "view in browser" boilerplate, `On … wrote:` reply attributions, forwarded-header blocks — are left to the application layer. +## Attachment filenames + +`parse_email` reports every part `name` already sanitized. +`sanitize_filename` is that same pass, exposed for names that never went +through the parser — a client-supplied one, say: + +```python +from jmap_email import sanitize_filename + +name = sanitize_filename(raw, max_length=255) or "unnamed" +``` + +It NFKC-normalizes, strips path components in both separator dialects +(`..\..\boot.ini` → `boot.ini`), removes every invisible character — +controls, bidi overrides, zero-width joiners, the BOM, lone surrogates — +replaces the characters filesystems reject, drops surrounding dots and +whitespace, and truncates while keeping the extension. The result is +safe to join onto a directory: no separator, no `..`, no leading dot (so +`.gitignore` comes back as `gitignore`), and NFKC-stable so normalizing +it later can't reintroduce any of those. + +It returns `None`, never `""`, when nothing usable survives, so the +failure case can't be mistaken for a name. Pass the `max_length` your +storage enforces; the 255 default is a convention, not a promise about +your column. + +Two of those steps are not cosmetic. The bidi strip: +`annexegpj.exe` renders as `annexe.exe.jpg` in most file pickers +— the user sees an image, the OS sees an executable, and it is the +standard attachment spoof. And normalizing *first*: `．．／etc／passwd` +is all fullwidth characters, so it passes any ASCII-based check +untouched, then folds to `../etc/passwd` as soon as something downstream +normalizes it. Sanitize-then-normalize is the bypass; normalize-then- +sanitize is not. + +A part carrying **no** filename is a different question. Per RFC 8621 +`name` is `String | null`, and `parse_email` reports `null` rather than +inventing a placeholder. What to show instead — `unnamed`, a +subject-derived name, an extension inferred from the part's MIME type — +is product policy, and stays in your application. + ## Validators Want to know if a string would be accepted by `compose_email` as a @@ -379,11 +625,30 @@ if is_valid_msg_id(parent_header): reply["inReplyTo"] = [parent_header] ``` -It applies exactly the same checks `compose_email` does — shape, -length ceiling, no embedded whitespace — but returns `True`/`False` +`is_valid_addr_spec` is its counterpart for addresses: `True` means one +well-formed mailbox — `local@domain`, no embedded whitespace, no comma +or semicolon splitting it in two, both halves non-empty — safe to put in +a header as it stands. A quoted-string local-part +(`"john doe"@example.com`) is accepted, since the quoting is what keeps +it a single mailbox. The parser and the composer share this predicate, +so a value one side calls an address is never one the other would mangle. + +It checks the shape a mailbox-list entry requires — one bare +`local@domain` addr-spec, nothing that would split it into two +mailboxes or need quoting on the way out — but returns `True`/`False` instead of raising. Useful for lenient parse paths (archive importers, -inbound salvaging) that need to decide between keeping a raw id and -falling back to synthesis without catching an exception. +inbound salvaging) that need to decide between keeping an address and +dropping it without catching an exception. + +**`True` means the value is usable as it stands.** The predicate +rejects anything `compose_email` would refuse — a line terminator, a +control character, an unquoted special. Answering `True` for +`"a@b.co\r\n"` would be handing back a header-injection payload to +anyone who writes the value somewhere other than `compose_email`. The +composer enforces the same rule: a malformed `email` value raises +`InvalidAddressError` rather than being silently cleaned, so the +predicate and the composer agree — `True` here is exactly the set of +values the composer emits unchanged. ## Strict vs. lenient `parse_address` @@ -406,12 +671,59 @@ fails the shape check are silently dropped — so `len(parse_addresses(header)) != header.count(",") + 1` is expected when the header carries garbage between real entries. +### Formatting addresses for display + +`format_address(name, email)` and `format_address_list(addresses)` go the +other way — from parsed values to a display string for a human, not a +header for the wire. + +```python +from jmap_email import format_address, format_address_list + +format_address("Alice", "alice@example.com") # "Alice " +format_address("Hara, Alice", "a@example.com") # '"Hara, Alice" ' +format_address("", "alice@example.com") # "alice@example.com" + +format_address_list(parsed["to"]) # "Alice , b@x.co" +``` + +A display name is quoted only when it needs to be, and an entry with no +name reduces to the bare addr-spec, so the output reads the way a mail +client shows it. + +`compose_email` already formats addresses for the messages it builds, so +these are not needed on the send path. They exist for text you write +*around* a message: the `From:`/`To:`/`Cc:` block of a forwarded-message +attribution, a "replying to" line, an audit log entry. + +`format_address_list` takes an `EmailAddress[]` — `parsed["to"]`, +`parsed["cc"]`, … — and returns `""` for an empty list. It does **not** +accept `None`, which is what those fields hold when the header is absent, +so guard with `parsed["cc"] or []`. + ## Defense matrix The parser explicitly defends against the documented attack classes below. See the `tests/` directory for regression coverage of each. -- **CVE-2023-27043** — `parseaddr`/`getaddresses` display-name confusion +- **CVE-2023-27043** — `parseaddr`/`getaddresses` display-name confusion, + in both its forms: the multi-tuple split (`"a@b.co" `, + where the authoritative angle-addr is taken rather than the first + tuple) and the unclosed-comment variant + (`victim@bank.com( `, where the comment eats the + angle-addr and leaves only the display-name-as-addr-spec — refused) +- **CVE-2019-16056** — multiple-`@` addr-spec: an allowlist keyed on the + domain talked into accepting one it meant to deny +- **CVE-2023-36632** — `parseaddr` `RecursionError` on nested comments +- **CVE-2026-30227** (MimeKit) — CR/LF in a quoted-string local-part; + the same shape reached this library as an addr-spec carrying a space + or a comma, which is two mailboxes rather than one +- **CVE-2023-51764** — SMTP smuggling. Some receivers accept `\n.\n` as + a DATA terminator, so a composer that emitted a bare LF would mint the + vector out of body text; our output is strictly CRLF, asserted +- **CVE-2025-52488** — Unicode normalization bypass: compatibility forms + that survive an ASCII check and fold to `../` afterwards + (`sanitize_filename` normalizes *before* it sanitizes) - **CVE-2024-6923** — header-injection via embedded newlines (compose) - **CVE-2024-21742** — Apache James `\r\n` in fields - **CVE-2024-23184** — Dovecot unbounded address-list allocation @@ -426,15 +738,18 @@ below. See the `tests/` directory for regression coverage of each. - **Mailsploit** — NUL-byte truncation in encoded-words - **USENIX 2020 "Weak Links in Auth Chains"** — duplicate `From:`, group-syntax, CFWS-in-address handling +- **CVE-2026-1299** — `BytesGenerator` header injection via unquoted + newlines. Fixed in CPython 3.14.3, below this package's 3.14.6 floor, + so the floor is what defends it — not our code ## Compatibility - **Python** 3.14.6+ (see [Why a Python 3.14.6 floor?](#why-a-python-3146-floor)) - **Platforms tested in CI:** Linux on x86_64 and arm64 - **macOS / Windows / PyPy / free-threaded build:** untested; expected - to work since the package has zero compiled extensions and zero - runtime dependencies. Reports of breakage welcome via the issue - tracker. + to work since the package has zero compiled extensions and a single + pure-Python runtime dependency (`idna`). Reports of breakage welcome + via the issue tracker. ## Performance and concurrency diff --git a/src/jmap-email/jmap_email/__init__.py b/src/jmap-email/jmap_email/__init__.py index 963a9023..9ab6bc12 100644 --- a/src/jmap-email/jmap_email/__init__.py +++ b/src/jmap-email/jmap_email/__init__.py @@ -23,6 +23,7 @@ __version__ = "0.2.0" # cluttering the top-level (runtime) API. Re-export the submodule itself so # ``jmap_email.types`` is always reachable and recognised as public. from . import types as types +from .addresses import is_valid_addr_spec from .composer import ( AttachmentError, ComposeError, @@ -35,6 +36,7 @@ from .composer import ( format_address_list, is_valid_msg_id, ) +from .filenames import sanitize_filename from .helpers import ( body_part_text, body_text_joined, @@ -49,7 +51,12 @@ from .helpers import ( now_sent_at, sent_at_to_datetime, ) -from .options import DEFAULT_PARSE_OPTIONS, ParseOptions +from .options import ( + DEFAULT_COMPOSE_OPTIONS, + DEFAULT_PARSE_OPTIONS, + ComposeOptions, + ParseOptions, +) from .parser import ( decode_rfc2047_header, parse_address, @@ -73,6 +80,10 @@ __all__ = [ "format_address_list", # Validators "is_valid_msg_id", + "is_valid_addr_spec", + # Attachment filename hygiene (applied by the parser to every part + # name it reports; public for names that never went through it) + "sanitize_filename", # Null-safe shape accessors "first_address", "first_address_email", @@ -90,6 +101,8 @@ __all__ = [ # Per-call resource caps "ParseOptions", "DEFAULT_PARSE_OPTIONS", + "ComposeOptions", + "DEFAULT_COMPOSE_OPTIONS", # Errors (compose-side only; parse_email returns None on failure) "ComposeError", "InvalidAddressError", diff --git a/src/jmap-email/jmap_email/addresses.py b/src/jmap-email/jmap_email/addresses.py new file mode 100644 index 00000000..985052a5 --- /dev/null +++ b/src/jmap-email/jmap_email/addresses.py @@ -0,0 +1,124 @@ +"""Addr-spec shape validation, shared by the parser and the composer. + +One mailbox, one predicate. The parse side uses it to decide whether +something the stdlib's lenient splitter surfaced is really an address; +the compose side uses it to refuse building a header out of one that +isn't. Keeping a single definition is the point — a value the parser +calls valid and the composer would mangle (or vice versa) is exactly +the gap an injection lives in. +""" + +# Characters the composer strips from every header value on the way out +# (``_sanitize_header_value``). Defined here because this predicate has to +# agree with it: a value carrying one of these is not usable as it stands, +# whatever its shape. TAB is absent deliberately — the composer keeps it, +# and the whitespace rule below rejects it inside an addr-spec anyway. +STRIPPED_HEADER_CHARS = frozenset( + [chr(c) for c in range(0x00, 0x20) if c != 0x09] + + ["\x7f", "\u0085", "\u2028", "\u2029"] +) + + +# RFC 5322 §3.2.3 ``specials``, minus ``.`` which is the dot-atom +# separator. Outside a quoted-string or a domain-literal each of these +# ends whatever token a reader is in the middle of: ``(`` opens a comment +# that runs to the closing paren, ``:`` opens a group, ``[`` opens a +# domain-literal, ``\\`` escapes the next character. A value carrying one +# is therefore not one mailbox to everybody — ``a(b@c.co, victim@x.co`` +# reads as *zero* addresses to a comment-aware parser, and ``a:b@c.co`` +# reads as the group ``a`` containing ``b@c.co``. +_SPECIALS = frozenset('()<>[]:;@\\,"') + +# RFC 5322 §3.4.1 ``dtext``: printable ASCII inside a domain-literal, +# excluding the framing brackets and the escape. The comma and the +# parens are legal dtext but are excluded anyway: this predicate +# promises one mailbox *to everybody*, and a lenient reader that does +# not track literal brackets cuts a mailbox-list at every comma and +# opens a comment at every ``(`` — ``email.utils.getaddresses`` +# recovers **zero** mailboxes from ``x@[a,b], victim@x.co`` and from +# ``x@[a(b], victim@x.co`` alike, the exact reader disagreement +# described above. +_DTEXT_EXCLUDED = frozenset("[]\\,()") + + +def _is_valid_domain(domain: str) -> bool: + """Return ``True`` when *domain* is a dot-atom or a domain-literal.""" + if domain.startswith("[") and domain.endswith("]") and len(domain) >= 2: + inner = domain[1:-1] + return not any(c.isspace() or c in _DTEXT_EXCLUDED for c in inner) + return not any(c.isspace() or c in _SPECIALS for c in domain) + + +def is_valid_addr_spec(addr: str | None) -> bool: + """Return ``True`` when *addr* is a single, well-formed addr-spec. + + RFC 5322 §3.4.1: ``local-part "@" domain``. The check that matters + here is that it is **one** mailbox — an ``email`` value carrying a + comma is two addresses, and a mailbox-list is built by joining on + commas, so letting one through turns a single recipient into two. + Whitespace is rejected for the same reason (a space separates a + display name from an angle-addr), as are ``<>;`` and an empty + local-part or domain. + + A quoted-string local-part (RFC 5322 §3.4.1 permits + ``"john doe"@example.com``) may contain those characters, since the + quoting is what keeps it one mailbox. Non-ASCII is accepted + throughout: RFC 6531 addresses are valid and this is not the place + to relitigate that. + + ``True`` means usable **as it stands**, so a value carrying a + character the composer would strip on the way out — a line + terminator, a control character — is rejected rather than reported + valid and silently cleaned later. Same rule as + :func:`jmap_email.is_valid_msg_id`: the caller keeps the raw string + it validated, and may write it somewhere other than + :func:`compose_email`. + """ + if not addr or "@" not in addr: + return False + if any(c in STRIPPED_HEADER_CHARS for c in addr): + return False + local, _, domain = addr.rpartition("@") + if not local or not domain: + return False + if not _is_valid_domain(domain): + return False + if len(local) >= 2 and local.startswith('"') and local.endswith('"'): + return _is_terminated_quoted_string(local) + return not any(c.isspace() or c in _SPECIALS for c in local) + + +def _is_terminated_quoted_string(local: str) -> bool: + """Return ``True`` when *local* is one complete quoted-string. + + RFC 5322 §3.2.4: ``DQUOTE *(qtext / quoted-pair) DQUOTE``, where a + quoted-pair is a backslash plus exactly one character. Escapes must + be walked left to right — stripping ``\\\\`` and ``\\"`` pairs and + checking for a leftover quote is not equivalent, because it accepts a + local-part ending in a lone backslash: in ``"a\\"`` that backslash + escapes the *closing* DQUOTE, so the string never terminates. + + Termination matters because the value is emitted into a header + verbatim. An unterminated quoted string keeps quoting whatever + follows it, so in a mailbox-list it swallows the comma before the + next recipient — and a reader that ends the string elsewhere than we + did counts a different number of mailboxes. That is the parser + disagreement the rest of this library exists to avoid. + """ + interior = local[1:-1] + index = 0 + while index < len(interior): + char = interior[index] + if char == "\\": + # A quoted-pair needs a character to escape. A backslash in + # final position has none left but the closing DQUOTE. + if index + 1 >= len(interior): + return False + index += 2 + continue + if char == '"': + # An unescaped quote ends the string early, and everything + # after it is outside the quoting that made this one mailbox. + return False + index += 1 + return True diff --git a/src/jmap-email/jmap_email/composer.py b/src/jmap-email/jmap_email/composer.py index 6c5d153a..0162b255 100644 --- a/src/jmap-email/jmap_email/composer.py +++ b/src/jmap-email/jmap_email/composer.py @@ -13,14 +13,26 @@ import binascii import datetime import logging import re +import secrets from email.errors import MessageError from email.generator import BytesGenerator from email.headerregistry import HeaderRegistry, UnstructuredHeader -from email.message import MIMEPart +from email.message import EmailMessage, MIMEPart from email.policy import SMTP as email_policy_smtp +from email.policy import EmailPolicy from email.utils import format_datetime, parsedate_to_datetime +from functools import lru_cache from io import BytesIO -from typing import Any +from typing import Any, cast + +import idna + +from .addresses import ( + STRIPPED_HEADER_CHARS, + _is_terminated_quoted_string, + is_valid_addr_spec, +) +from .options import DEFAULT_COMPOSE_OPTIONS, ComposeOptions logger = logging.getLogger(__name__) @@ -63,6 +75,27 @@ _HEADER_FACTORY.map_to_type("references", UnstructuredHeader) # ty: ignore[inva _POLICY = email_policy_smtp.clone(cte_type="7bit", header_factory=_HEADER_FACTORY) +@lru_cache(maxsize=4) +def _policy_for(cte_type: str, utf8: bool) -> EmailPolicy[EmailMessage]: + """Return the serialization policy for one (8BITMIME, SMTPUTF8) pair. + + Four combinations exist and each is a small immutable object, so they + are cached rather than cloned per call. ``_HEADER_FACTORY`` is shared + deliberately: it is our own dedicated instance (see above), and every + variant needs the same In-Reply-To / References mapping. + """ + if cte_type == "7bit" and not utf8: + return _POLICY + return email_policy_smtp.clone( + cte_type=cte_type, utf8=utf8, header_factory=_HEADER_FACTORY + ) + + +def _policy_for_options(options: ComposeOptions) -> EmailPolicy[EmailMessage]: + """Pick the policy an options bundle asks for.""" + return _policy_for("8bit" if options.emits_8bit else "7bit", options.allow_smtputf8) + + class ComposeError(Exception): """Base class for all composer errors. @@ -134,6 +167,127 @@ _RESERVED_HEADER_NAMES = frozenset( ) +def _idna_encode_domain(domain: str) -> str | None: + """Return the A-label (punycode) form of *domain*, or ``None``. + + ``None`` means the domain has no ASCII form we are willing to emit: + a label over 63 octets, an empty label (``a..b.fr``), a code point + IDNA2008 disallows, or a trailing root dot — ``idna`` would keep + the dot of ``exemplé.fr.``, but a domain ending in ``.`` is not a + valid RFC 5322 dot-atom, so it is refused here instead. + + The ``idna`` package (UTS 46, non-transitional) rather than the + stdlib codec — the library's one runtime dependency. The stdlib is + IDNA2003, whose nameprep *folds* what it does not refuse: + ``faß.de`` became ``fass.de`` — a distinct registrable domain + since DENIC allowed ß in 2010 (likewise the Greek final sigma) — + so mail was silently routed to the folded sibling. UTS 46 + non-transitional preserves those code points, matching what + browsers and modern resolvers look up. + """ + if domain.endswith("."): + return None + try: + return idna.encode(domain, uts46=True).decode("ascii") + except idna.IDNAError: + return None + + +def _normalize_addr_list(addr_list: Any, *, field: str, options: ComposeOptions) -> Any: + """Validate every mailbox in *addr_list*, IDNA-encoding where asked. + + Returns the list to compose from: *addr_list* itself when nothing + needed rewriting, otherwise a new list of new dicts. The caller's + input is never mutated — a JMAP dict handed to ``compose_email`` is + frequently the same object the caller goes on to store. + + An entry with no ``email`` is left alone; the composer already treats + those as absent. What this refuses is a present-but-malformed one, + most importantly an ``email`` containing a comma: joined into a + mailbox-list it becomes two recipients, which is address injection + performed by the library on its caller's behalf. + """ + if not isinstance(addr_list, list): + return addr_list + rewritten: dict[int, dict[str, Any]] = {} + for index, raw_entry in enumerate(addr_list): + if not isinstance(raw_entry, dict): + continue + # ``addr_list`` is ``Any``, so ``isinstance`` narrows only to an + # un-parameterised dict; name the shape we actually require. + entry = cast(dict[str, Any], raw_entry) + raw = entry.get("email") + # Checked as supplied, only surrounding whitespace removed. Running + # it through ``_sanitize_header_value`` first would let a control + # character be quietly deleted and the survivor emitted — a + # recipient the caller never wrote. Strict compose means that is an + # error, like any other malformed addr-spec. + email = str(raw or "").strip() + if not email: + continue + if not is_valid_addr_spec(email): + raise InvalidAddressError( + f"Invalid addr-spec in {field!r}: {raw!r} is not a single mailbox" + ) + if email.isascii(): + continue + local, _, domain = email.rpartition("@") + # Under SMTPUTF8 the whole addr-spec may travel as UTF-8, so both + # halves are already legal and there is nothing to convert. + if options.allow_smtputf8: + continue + # Otherwise the local part is checked first, and separately. + # Punycode is a DNS algorithm; a local part is not a DNS label, so + # there is no ASCII form to convert it to — only SMTPUTF8 carries + # it, and ``idna_encode_domains`` deliberately does not reach here. + if not local.isascii(): + raise InvalidAddressError( + f"Non-ASCII local-part in {field!r}: {raw!r} needs SMTPUTF8 " + "(RFC 6531); pass options=ComposeOptions(allow_smtputf8=True) only " + "if the next hop advertised it" + ) + if not options.idna_encode_domains: + raise InvalidAddressError( + f"Non-ASCII domain in {field!r}: {raw!r} must be IDNA encoded " + "to travel over 7-bit SMTP (pass " + "options=ComposeOptions(idna_encode_domains=True) to have the " + "composer do it)" + ) + encoded = _idna_encode_domain(domain) + if encoded is None: + raise InvalidAddressError( + f"Non-ASCII domain in {field!r}: {raw!r} has no IDNA encoding" + ) + rewritten[index] = {**entry, "email": f"{local}@{encoded}"} + if not rewritten: + return addr_list + return [rewritten.get(i, entry) for i, entry in enumerate(addr_list)] + + +# The JMAP address-list fields, in the order the wire header block wants +# them. Every one is validated before any of them is formatted. +_ADDR_LIST_FIELDS = ("from", "sender", "replyTo", "to", "cc", "bcc") + + +def _normalize_addresses( + jmap_data: dict[str, Any], options: ComposeOptions +) -> dict[str, Any]: + """Return *jmap_data* with every address list validated and normalized. + + A shallow copy is made only if some list actually changed, so the + common all-ASCII path hands the original dict straight through. + """ + replacements = {} + for field in _ADDR_LIST_FIELDS: + original = jmap_data.get(field) + normalized = _normalize_addr_list(original, field=field, options=options) + if normalized is not original: + replacements[field] = normalized + if not replacements: + return jmap_data + return {**jmap_data, **replacements} + + def format_address(name: str, email: str) -> str: """Format a name and email address according to RFC 5322. @@ -150,15 +304,40 @@ def format_address(name: str, email: str) -> str: >>> format_address('John Doe', 'john@example.com') 'John Doe ' """ - email = _sanitize_header_value(email or "") - if not email: + email = (email or "").strip() + # Shape as well as injection: an addr-spec carrying a comma would be + # joined into a mailbox-list as two recipients. A display helper must + # not be the thing that mints a second mailbox. + if not is_valid_addr_spec(email): return "" name = _sanitize_header_value(name or "") if not name: return email.strip() - needs_quoting = any(c in name for c in ',.;:@<>()[]"\\') - if needs_quoting and not (name.startswith('"') and name.endswith('"')): + # The header machinery decodes RFC 2047 encoded-words *after* this + # decision, so the quoting question has to be asked about the text + # that will actually be emitted, not the literal we were handed. + # ``=?utf-8?B?ZXZpbEB4LmNv?=`` carries none of the specials below, + # so it went out unquoted — and decoded to ``evil@x.co``, giving + # ``To: evil@x.co ``, which is two mailboxes. A display name + # an attacker chose thereby became a recipient. + looks_encoded = "=?" in name and "?=" in name + needs_quoting = looks_encoded or any(c in name for c in ',.;:@<>()[]"\\') + # "Already quoted" has to mean a *complete* quoted-string, not merely + # a leading and a trailing DQUOTE. A lone ``"`` satisfies both + # ``startswith`` and ``endswith`` at once, ``"a"b"`` closes early, and + # ``"a\\"`` ends on a backslash that escapes its own closing quote. + # Emitting any of those verbatim unbalances the header, and in a + # mailbox-list the next entry's display name is then read as an + # address — the display-name-becomes-recipient bug, arrived at from + # the quoting side rather than the encoded-word side. + already_quoted = ( + len(name) >= 2 + and name.startswith('"') + and name.endswith('"') + and _is_terminated_quoted_string(name) + ) + if needs_quoting and not already_quoted: # RFC 5322 §3.2.4: quoted-pair escapes the next character, so the # backslash must be doubled before any embedded ``"`` is escaped — # otherwise ``a\"`` round-trips as ``a"`` and quoted-pair sequences @@ -168,10 +347,19 @@ def format_address(name: str, email: str) -> str: return f"{name} <{email.strip()}>" -def format_address_list(addresses: list[dict[str, str]]) -> str: - """Format a list of address dicts as a comma-separated RFC 5322 mailbox-list.""" +def format_address_list(addresses: Any) -> str: + """Format a list of address dicts as a comma-separated RFC 5322 mailbox-list. + + Non-dict entries are skipped, as in ``_normalize_addr_list``: the shape + comes from caller JSON, so a stray ``null`` is malformed input, not a + reason to raise out of ``.get``. + """ + if not isinstance(addresses, list): + return "" formatted = [] for addr in addresses: + if not isinstance(addr, dict): + continue name = addr.get("name", "") email = addr.get("email", "") if email: @@ -191,9 +379,7 @@ def format_address_list(addresses: list[dict[str, str]]) -> str: # in receivers that interpret e.g. \x01 (SOH) as a separator. Stripping # them silently is consistent with our "compose strict, parse lenient" # contract. TAB stays \u2014 it's legal FWS. -_HEADER_INJECTION_CHARS = ( - "".join(chr(c) for c in range(0x00, 0x20) if c != 0x09) + "\x7f\u0085\u2028\u2029" -) +_HEADER_INJECTION_CHARS = "".join(sorted(STRIPPED_HEADER_CHARS)) _HEADER_INJECTION_TABLE = str.maketrans("", "", _HEADER_INJECTION_CHARS) @@ -229,21 +415,33 @@ _MSG_ID_MAX_OCTETS = 900 def is_valid_msg_id(value: str | None) -> bool: """Return True when ``value`` matches the composer's msg-id shape. - The same predicate :func:`compose_email` applies to Message-ID / - In-Reply-To / References entries: ````, no internal - whitespace, no nested angle brackets, at least one ``@``, and - within the ``_MSG_ID_MAX_OCTETS`` byte ceiling. Angle brackets are - optional \u2014 callers may pass either the stripped (``local@domain``) - or wrapped (````) form. + The shape :func:`compose_email` requires of Message-ID / In-Reply-To + / References entries: ````, no internal whitespace, no + nested angle brackets, at least one ``@``, and within the + ``_MSG_ID_MAX_OCTETS`` byte ceiling. Angle brackets are optional + \u2014 callers may pass either the stripped (``local@domain``) or + wrapped (````) form. Use this from lenient-parse paths (archive importers, inbound salvaging) to decide whether to keep a raw msg-id or fall back to synthesis \u2014 checking the predicate yourself rather than try/except against :func:`compose_email` keeps the cold path cheap. + + ``True`` means the value is usable **as it stands**. A value + carrying characters the composer would strip on the way out \u2014 a + line terminator, a control character \u2014 is rejected rather than + silently cleaned, because the caller keeps the raw string it + validated: answering ``True`` for ``""`` would hand a + header-injection payload to anyone who writes that value somewhere + other than :func:`compose_email`. The composer itself stays lenient + and sanitizes such an entry instead of rejecting the whole message, + so this predicate is the stricter of the two by design. """ if not isinstance(value, str) or not value: return False - cleaned = _ensure_angle_brackets(_sanitize_header_value(value)) + if _sanitize_header_value(value) != value: + return False + cleaned = _ensure_angle_brackets(value) if len(cleaned.encode("utf-8", errors="replace")) > _MSG_ID_MAX_OCTETS: return False return _MSG_ID_RE.match(cleaned) is not None @@ -372,25 +570,6 @@ def _first_msgid(value: list[str] | None) -> str | None: return None -def _collect_msgids(value: list[str] | None) -> str: - """Join a JMAP ``MessageIds`` ``String[]`` into a single - space-separated chain (angle-bracket form) for the wire. - - Strict-typed: see :func:`_first_msgid` — only accepts a list. - """ - if not isinstance(value, list) or not value: - return "" - chain: list[str] = [] - for v in value: - if not isinstance(v, str) or not v: - continue - sanitized = v.strip() - if not (sanitized.startswith("<") and sanitized.endswith(">")): - sanitized = f"<{sanitized}>" - chain.append(sanitized) - return " ".join(chain) - - def _iter_custom_headers( jmap_headers: list[dict[str, str]] | None, ) -> list[tuple[str, str]]: @@ -526,14 +705,14 @@ def _set_basic_headers( # pylint: disable=too-many-branches message_part: MIMEPart, jmap_data: dict[str, Any], in_reply_to: str | None = None, - keep_bcc: bool = False, + emit_bcc: bool = False, ) -> None: """Set the basic email headers on a message part. - keep_bcc: if False (default), Bcc in jmap_data is dropped — the entire + emit_bcc: if False (default), Bcc in jmap_data is dropped — the entire point of Bcc is that recipients don't see each other. Only callers that are reconstructing an archive (e.g. PST import, where the Bcc list was - already in the source file) should pass keep_bcc=True. + already in the source file) should pass emit_bcc=True. """ # MIME-Version is required on every top-level MIME message (RFC 2045 §4). # MIMEPart — unlike EmailMessage — does NOT add it implicitly, so we set @@ -547,7 +726,12 @@ def _set_basic_headers( # pylint: disable=too-many-branches # ``from``: JMAP ``EmailAddress[]``. Emit the first author as # the ``From:`` header (multi-author mailbox-lists are rare and - # most receivers reject them anyway). + # most receivers reject them anyway). Every address list reaching + # here has already been shape-checked and normalized by + # ``_normalize_addresses`` in ``compose_email`` — before any of them + # was formatted, so a malformed addr-spec is an error rather than + # something we quietly drop (losing a recipient) or quietly emit + # (gaining one). from_data = jmap_data.get("from") first_from = _first_address(from_data) if first_from: @@ -578,7 +762,7 @@ def _set_basic_headers( # pylint: disable=too-many-branches # is non-empty. An empty list of valid addresses must NOT produce # an empty To: header (most receivers reject). recipient_fields = [("to", "To"), ("cc", "Cc")] - if keep_bcc: + if emit_bcc: recipient_fields.append(("bcc", "Bcc")) for jmap_key, header_name in recipient_fields: addr_list = jmap_data.get(jmap_key) @@ -709,7 +893,9 @@ def _normalize_cid(cid: str) -> str: _CID_STRUCTURAL_RE = re.compile(r"^<[^\s<>]+>$") -def _create_attachment_part(attachment: dict[str, Any]) -> MIMEPart: +def _create_attachment_part( + attachment: dict[str, Any], options: ComposeOptions +) -> MIMEPart: """Create a MIME part for an attachment from JMAP data. Strict-by-design: the composer is caller-controlled and refuses to @@ -780,7 +966,7 @@ def _create_attachment_part(attachment: dict[str, Any]) -> MIMEPart: maintype, subtype = "text", "plain" try: - part = MIMEPart(policy=_POLICY) + part = MIMEPart(policy=_policy_for_options(options)) kwargs: dict[str, Any] = { "maintype": maintype, "subtype": subtype, @@ -837,6 +1023,9 @@ def _build_body(msg: MIMEPart, jmap_data: dict[str, Any]) -> None: if text_body is not None and html_body is not None: msg.set_content(text_body, subtype="plain", charset="utf-8") msg.add_alternative(html_body, subtype="html", charset="utf-8") + # ``add_alternative`` converted ``msg`` to multipart/alternative and + # left the boundary to the stdlib's non-CSPRNG generator. + msg.set_boundary(_fresh_boundary()) elif text_body is not None: msg.set_content(text_body, subtype="plain", charset="utf-8") elif html_body is not None: @@ -846,7 +1035,9 @@ def _build_body(msg: MIMEPart, jmap_data: dict[str, Any]) -> None: def _wrap_with_inline_images( - body_part: MIMEPart, inline_attachments: list[dict[str, Any]] + body_part: MIMEPart, + inline_attachments: list[dict[str, Any]], + options: ComposeOptions, ) -> MIMEPart: """Wrap a body part with multipart/related to attach inline images by cid. @@ -855,15 +1046,16 @@ def _wrap_with_inline_images( MIMEPart._make_multipart's `disallowed_subtypes`. Wrapping a fresh related part around the existing body bypasses that check. - If every inline attachment fails to build, the body is returned - unwrapped: a single-child multipart/related is wasteful and confuses - some receivers. + Callers only reach this with a non-empty list, and + ``_create_attachment_part`` raises rather than returning a falsy part + for every bad input, so ``built`` always has at least one entry — + ``AttachmentError`` and ``InvalidMessageIdError`` propagate to the + caller instead of the attachment being silently dropped. """ - built = [p for p in (_create_attachment_part(a) for a in inline_attachments) if p] - if not built: - return body_part - related = MIMEPart(policy=_POLICY) + built = [_create_attachment_part(a, options) for a in inline_attachments] + related = MIMEPart(policy=_policy_for_options(options)) related.make_related() + related.set_boundary(_fresh_boundary()) # RFC 2387 §3.1 requires the multipart/related Content-Type to carry a # ``type=`` parameter naming the root part's media type. Without it, # downstream MUAs that follow the spec fall back to alternate rendering @@ -875,19 +1067,39 @@ def _wrap_with_inline_images( return related +def _fresh_boundary() -> str: + """Return an unpredictable MIME boundary. + + The stdlib generates boundaries with ``random.randrange`` — a + Mersenne Twister, not a CSPRNG. Boundaries are visible in every + message a system emits, and MT19937 state is recoverable from + enough consecutive outputs, so a party who can collect sequential + boundaries (by provoking autoreplies, say) can predict the next + one. Predicting it is what makes MIME part injection possible: a + sender who also controls a slice of the body — quoted text in an + autoreply, an echoed subject — can close the part we opened and + append parts of their own, which the recipient's client renders as + ours. + + The alphabet is a subset of RFC 2046 ``bcharsnospace``. + """ + return f"=_{secrets.token_urlsafe(24)}" + + def _wrap_with_attachments( - body_part: MIMEPart, regular_attachments: list[dict[str, Any]] + body_part: MIMEPart, + regular_attachments: list[dict[str, Any]], + options: ComposeOptions, ) -> MIMEPart: """Wrap a body part with multipart/mixed and append regular attachments. - Same fresh-wrapper pattern as _wrap_with_inline_images; if every - attachment fails to build, the body is returned unwrapped. + Same fresh-wrapper pattern as _wrap_with_inline_images, and the same + strict contract: a bad attachment raises rather than being dropped. """ - built = [p for p in (_create_attachment_part(a) for a in regular_attachments) if p] - if not built: - return body_part - mixed = MIMEPart(policy=_POLICY) + built = [_create_attachment_part(a, options) for a in regular_attachments] + mixed = MIMEPart(policy=_policy_for_options(options)) mixed.make_mixed() + mixed.set_boundary(_fresh_boundary()) mixed.attach(body_part) for att_part in built: mixed.attach(att_part) @@ -897,7 +1109,7 @@ def _wrap_with_attachments( def _create_multipart_message( jmap_data: dict[str, Any], in_reply_to: str | None = None, - keep_bcc: bool = False, + options: ComposeOptions = DEFAULT_COMPOSE_OPTIONS, ) -> MIMEPart: """Create the top-level MIMEPart from JMAP data. @@ -928,19 +1140,24 @@ def _create_multipart_message( inline_attachments: list[dict[str, Any]] = [] regular_attachments: list[dict[str, Any]] = [] for a in jmap_data.get("attachments", []) or []: + # Refused, not dropped: losing an attachment silently is data loss. + if not isinstance(a, dict): + raise AttachmentError( + f"Attachment must be an object, got {type(a).__name__}" + ) if a.get("disposition") == "inline" and a.get("cid"): inline_attachments.append(a) else: regular_attachments.append(a) - msg = MIMEPart(policy=_POLICY) + msg = MIMEPart(policy=_policy_for_options(options)) _build_body(msg, jmap_data) if inline_attachments: - msg = _wrap_with_inline_images(msg, inline_attachments) + msg = _wrap_with_inline_images(msg, inline_attachments, options) if regular_attachments: - msg = _wrap_with_attachments(msg, regular_attachments) + msg = _wrap_with_attachments(msg, regular_attachments, options) - _set_basic_headers(msg, jmap_data, in_reply_to, keep_bcc=keep_bcc) + _set_basic_headers(msg, jmap_data, in_reply_to, emit_bcc=options.emit_bcc) return msg @@ -949,8 +1166,7 @@ def compose_email( *, in_reply_to: str | None = None, prepend_headers: list[tuple[str, str]] | None = None, - keep_bcc: bool = False, - allow_extensions: bool = True, + options: ComposeOptions | None = None, ) -> bytes: """Compose a JMAP Email object dict into RFC 5322 bytes. @@ -970,15 +1186,14 @@ def compose_email( prepend_headers : list of (name, value), optional Extra headers to inject at the top of the output (e.g. ``Received:`` set by an MTA-out pipeline). - keep_bcc : bool, default False - When False, the ``Bcc:`` header is silently dropped — the - entire point of Bcc is that it must NOT be transmitted to - recipients. Set True only for archive-reconstruction use - cases (e.g. PST import). - allow_extensions : bool, default True - When False, any ``_ext`` key in ``jmap_data`` raises - ``ComposeError`` — a strict-JMAP signal that the caller is - not silently relying on project extensions. + options : ComposeOptions, optional + Output policy: ``emit_bcc``, ``idna_encode_domains``, + ``allow_8bit``, ``allow_smtputf8``. See :class:`jmap_email.ComposeOptions`. + Defaults to :data:`jmap_email.DEFAULT_COMPOSE_OPTIONS`. + + The two parameters above stay separate because they are + per-*message* data that changes on every call; everything in + ``options`` is a property of the call site, set once. Note on dot-stuffing: this function produces RFC 5322 bytes; it does NOT apply RFC 5321 §4.5.2 dot-stuffing. Callers that hand the @@ -990,16 +1205,12 @@ def compose_email( ComposeError If composition fails. """ + if options is None: + options = DEFAULT_COMPOSE_OPTIONS try: if not jmap_data: raise ComposeError("Empty JMAP data provided") - if not allow_extensions and "_ext" in jmap_data: - raise ComposeError( - "Strict-JMAP input rejects ``_ext`` key " - "(pass allow_extensions=True to accept project extensions)" - ) - # ``from`` must be a non-empty ``EmailAddress[]`` (RFC 8621 §4.1.2) # with at least one entry carrying a non-empty ``email``. from_data = jmap_data.get("from") @@ -1009,7 +1220,11 @@ def compose_email( if not first_from or not first_from.get("email"): raise InvalidAddressError("Missing or invalid 'from' field in JMAP data") - msg = _create_multipart_message(jmap_data, in_reply_to, keep_bcc=keep_bcc) + # Shape-check and normalize every address list up front, so the + # whole MIME tree below is built from validated mailboxes. + jmap_data = _normalize_addresses(jmap_data, options) + + msg = _create_multipart_message(jmap_data, in_reply_to, options) if prepend_headers: # Insert at the top of the header block so they appear before @@ -1043,7 +1258,7 @@ def compose_email( out = BytesIO() # ``BytesGenerator.flatten`` accepts any ``Message`` subclass at # runtime; the stub narrows to ``EmailMessage``. - BytesGenerator(out, policy=_POLICY).flatten(msg) # ty: ignore[invalid-argument-type] + BytesGenerator(out, policy=_policy_for_options(options)).flatten(msg) # ty: ignore[invalid-argument-type] return out.getvalue() except ComposeError: # pylint: disable=try-except-raise diff --git a/src/jmap-email/jmap_email/filenames.py b/src/jmap-email/jmap_email/filenames.py new file mode 100644 index 00000000..8cf87674 --- /dev/null +++ b/src/jmap-email/jmap_email/filenames.py @@ -0,0 +1,132 @@ +"""Attachment filename hygiene. + +:func:`sanitize_filename` defangs a filename that arrived over the wire — +path components, invisible characters, length — while keeping it +recognizable. ``parse_email`` applies it to every part name it reports; +it is public so consumers can apply it to names that never went through +the parser, such as client-supplied ones. + +Naming a part that carries *no* filename is deliberately not covered +here. Per RFC 8621 ``name`` is ``String | null`` and ``parse_email`` +reports ``null`` rather than inventing a placeholder — what to display +instead is consumer policy. +""" + +import re +import unicodedata +from ntpath import basename as nt_basename +from posixpath import basename as posix_basename + +# Unicode categories removed outright. Deliberately broad: a filename is +# shown to a human and handed to a filesystem, and each of these is +# invisible to the first while meaning something to the second. +# +# Cc controls (NUL, CR, LF, TAB, DEL, the C1 block) +# Cf format characters — bidi overrides (U+202E renders "annexe.exe" +# as "annexe.txt", the classic attachment spoof), zero-width +# joiners and spaces, the BOM +# Zl U+2028 line separator +# Zp U+2029 paragraph separator +# Cs lone surrogates — unencodable, they raise on write +# +# The cost is that a ZWJ emoji sequence degrades to its component emoji: +# a cosmetic loss on a filename, against an allowlist that would need +# revisiting every Unicode release. +_STRIPPED_CATEGORIES = frozenset({"Cc", "Cf", "Zl", "Zp", "Cs"}) + +# Replaced with "_": legal in a name but reserved by some filesystem or +# shell, so they stay visible rather than vanishing. +_UNSAFE_CHARS_RE = re.compile(r'[<>:"|?*\\/]') + +# Stripped from both ends: quote framing left by a MIME parameter, the +# dot/separator runs that spell "." and "..", and surrounding whitespace +# (Windows silently drops trailing dots and spaces, so "x.exe " and +# "x.exe" name the same file there). +_FRAMING_CHARS = '"/.\\ ' + + +def sanitize_filename(filename: str | None, max_length: int = 255) -> str | None: + """Sanitize an attachment filename, preserving the extension when truncating. + + Strips path components (POSIX and Windows both, since the wire does + not say which system produced the name), every invisible character, + and the characters filesystems reject; then truncates to *max_length* + keeping a reasonable extension intact. Pass the limit your storage + enforces rather than relying on the default. + + Returns ``None`` — never ``""`` — when nothing usable survives, so + the failure case can never be mistaken for a name. Callers wanting a + placeholder write ``sanitize_filename(x) or "unnamed"``. + + The result is safe to join onto a directory: it holds no separator, + no traversal segment, and no leading dot (so ``.gitignore`` comes + back as ``gitignore``). It is also NFKC-stable, so normalizing it + downstream cannot reintroduce any of those. + + What it does **not** do is apply the naming policy of whatever + filesystem you are about to write to. A name can be perfectly clean + and still mean something particular there — ``nul.txt`` is the null + device on Windows 10, ``aux`` on every Windows version — and the + right answer depends on the target OS, which a parser cannot see. + That check belongs where the file is opened; see + ``werkzeug.utils.secure_filename`` for the shape of it. + """ + if not filename or max_length <= 0: + return None + + # Bound the work before normalizing: NFKC expands by up to 18x + # (U+FDFA), so an unbounded caller-supplied name is a memory + # multiplier. Everything past this is discarded by the truncation + # below anyway, with slack far beyond any composition's ability to + # shrink text back under *max_length*. + filename = filename[: max_length * 32] + + # The next two steps are ordered, and it matters in both directions. + # + # Invisibles go first. A format character with combining class 0 sits + # *outside* a run of dots and shields it from the strip further down; + # deleting it afterwards re-exposes them, so ``"\x00..\x00"`` would + # come back as ``".."`` — the parent directory, intact. + filename = "".join( + c for c in filename if unicodedata.category(c) not in _STRIPPED_CATEGORIES + ) + + # Then compatibility-normalize, before anything looks for a separator. + # Sanitizing and *then* normalizing is a known bypass (CVE-2025-52488 + # and relatives): U+FF0F FULLWIDTH SOLIDUS and U+FF0E FULLWIDTH FULL + # STOP survive any ASCII-based check untouched, then NFKC folds them + # to "/" and "." — so a name we called clean becomes "../etc/passwd" + # the moment a database collation, a macOS filesystem or a caller's + # own normalize() touches it. U+2026 folds to "..." the same way. + # + # Doing it *after* the strip above is what keeps the result a fixed + # point. Those format characters block canonical composition, so + # deleting them can leave a base and its combining mark composable: + # normalizing first and deleting after returned a name that NFKC + # still had work to do on. Nothing below this line removes a + # non-ASCII character, and NFKC provably never emits a character in + # ``_STRIPPED_CATEGORIES`` (checked across all 0x110000 code points), + # so a single pass in this order is sufficient. + filename = unicodedata.normalize("NFKC", filename) + + filename = nt_basename(posix_basename(filename)) + + filename = filename.strip(_FRAMING_CHARS) + filename = _UNSAFE_CHARS_RE.sub("_", filename) + + if len(filename) > max_length: + truncated = filename[:max_length] + # Keep the extension when there is one worth keeping: a dot that + # isn't leading, short enough to be an extension, and leaving room + # for at least one character of name. + last_dot = filename.rfind(".") + if last_dot > 0: + ext = filename[last_dot:] + if len(ext) <= 10 and max_length - len(ext) > 0: + truncated = filename[: max_length - len(ext)] + ext + # Cutting can expose a new trailing dot (``"ab.cd"`` capped at 3 + # gives ``"ab."``), so strip once more — otherwise sanitizing an + # already-sanitized name would keep changing it. + filename = truncated.strip(_FRAMING_CHARS) + + return filename or None diff --git a/src/jmap-email/jmap_email/helpers.py b/src/jmap-email/jmap_email/helpers.py index deb78792..5319fc06 100644 --- a/src/jmap-email/jmap_email/helpers.py +++ b/src/jmap-email/jmap_email/helpers.py @@ -5,7 +5,10 @@ The wire shape uses lists everywhere (``from: list[EmailAddress]``, helpers wrap the first-element / case-insensitive lookup patterns so consumers don't repeat ``parsed.get("from") or []`` + index + ``.get`` chains. Every helper returns a sensible default on absence; none of -them ever raises. +them ever raises — including on ``None``, which is what +:func:`jmap_email.parse_email` hands back for input it cannot parse at +all, so ``find_header(parse_email(raw), "Subject")`` is safe to write +without an intervening ``is None`` check. These helpers complement :func:`jmap_email.parse_email` and live in the same package so that one ``pip install jmap-email`` ships everything @@ -16,6 +19,8 @@ strict-JMAP, and the accessors stay null-safe. from datetime import datetime, timezone from typing import Any +from .addresses import STRIPPED_HEADER_CHARS + __all__ = [ "first_address", "first_address_email", @@ -50,9 +55,12 @@ def now_sent_at() -> str: def first_address(addrs: Any) -> dict[str, Any] | None: """Return the first entry of a JMAP ``EmailAddress[]`` or ``None``. - An entry without an ``email`` is treated as missing. + An entry without an ``email`` is treated as missing. Strict-typed: + see :func:`first_msgid` — only a ``list`` is accepted, so a scalar + is rejected rather than iterated (a bare ``str`` would otherwise be + walked character by character, and a non-iterable would raise). """ - if not addrs: + if not isinstance(addrs, list) or not addrs: return None for entry in addrs: if isinstance(entry, dict) and entry.get("email"): @@ -92,7 +100,12 @@ def first_msgid(ids: Any) -> str: def msgid_chain(ids: Any) -> str: """Reassemble a JMAP ``String[]`` of msg-ids into the angle-bracketed space-separated wire form (e.g. ``" "``). Strict-typed: - see :func:`first_msgid` — only a list of strings is accepted.""" + see :func:`first_msgid` — only a list of strings is accepted. + + An entry that cannot be written into a header as it stands — a line + terminator, internal whitespace, a nested angle bracket — is dropped + rather than emitted, since the result is meant to go straight into + ``References``/``In-Reply-To``.""" if not isinstance(ids, list) or not ids: return "" out: list[str] = [] @@ -100,36 +113,95 @@ def msgid_chain(ids: Any) -> str: if not isinstance(v, str) or not v: continue sanitized = v.strip() - if not (sanitized.startswith("<") and sanitized.endswith(">")): - sanitized = f"<{sanitized}>" - out.append(sanitized) + if sanitized.startswith("<") and sanitized.endswith(">"): + sanitized = sanitized[1:-1] + # This helper exists to be written into a header, so it is the + # one place that must not hand back something unwritable. A line + # terminator is header injection outright; internal whitespace + # folds mid-id and downstream MID parsers truncate at the fold, + # silently corrupting the thread; a nested angle bracket makes + # the token boundaries ambiguous. Malformed entries are dropped + # rather than emitted, matching what the composer does with a + # bad References entry. + if any( + c.isspace() or c in STRIPPED_HEADER_CHARS or c in "<>" for c in sanitized + ): + continue + if not sanitized: + continue + out.append(f"<{sanitized}>") return " ".join(out) +def _as_aware(value: datetime) -> datetime: + """Treat a naive datetime as UTC. + + ``datetime.fromisoformat`` returns a *naive* object for an input + carrying no offset (``"2026-01-01T00:00:00"``, or a bare date), and + mixing one of those into a comparison with an aware datetime raises + ``TypeError`` at the call site rather than here. RFC 8621 ``UTCDate`` + always carries an offset, so an input without one is already outside + the spec; UTC is the same assumption :func:`compose_email` makes. + """ + if value.tzinfo is None or value.utcoffset() is None: + return value.replace(tzinfo=timezone.utc) + return value + + def sent_at_to_datetime(sent_at: Any) -> datetime | None: """Parse a JMAP ``sentAt`` ISO-8601 string into a tz-aware :class:`datetime`. Returns ``None`` on absence or parse failure. - A ``datetime`` instance is returned as-is so callers can pass - either shape through unchanged.""" + A ``datetime`` instance is passed through, naive ones stamped UTC + so the return type is uniformly aware.""" if not sent_at: return None if isinstance(sent_at, datetime): - return sent_at + return _as_aware(sent_at) if isinstance(sent_at, str): try: - return datetime.fromisoformat(sent_at) + return _as_aware(datetime.fromisoformat(sent_at)) except (TypeError, ValueError): return None return None +def _header_entries(parsed_email: Any) -> list[Any]: + """Return the ``headers`` list, or ``[]`` for anything unusable. + + ``parsed_email.get("headers") or []`` is not enough: a truthy + non-iterable (``5``, ``True``, ``3.5``) passes the ``or`` and then + raises ``TypeError`` on the ``for``. These accessors promise never to + raise, and they are exactly what a caller reaches for *before* + checking anything, so the type has to be established rather than + assumed. + """ + if not isinstance(parsed_email, dict): + return [] + entries = parsed_email.get("headers") + return entries if isinstance(entries, list) else [] + + +def _entry_name_matches(entry: Any, target: str) -> bool: + """True when *entry* is a header dict whose name equals *target*. + + ``entry.get("name")`` can be any JSON value, and a non-string one has + no ``.lower()``. Compared as a string so a numeric name simply fails + to match instead of raising. + """ + if not isinstance(entry, dict): + return False + name = entry.get("name") + return isinstance(name, str) and name.lower() == target + + def find_header(parsed_email: dict[str, Any], name: str) -> str: """Return the value of the first header whose name matches ``name`` case-insensitively, or ``""`` when absent.""" target = name.lower() - for entry in parsed_email.get("headers") or []: - if isinstance(entry, dict) and (entry.get("name") or "").lower() == target: - return entry.get("value") or "" + for entry in _header_entries(parsed_email): + if _entry_name_matches(entry, target): + value = entry.get("value") + return value if isinstance(value, str) else "" return "" @@ -138,9 +210,9 @@ def find_headers(parsed_email: dict[str, Any], name: str) -> list[str]: (case-insensitive), in document order. Empty list when absent.""" target = name.lower() return [ - entry.get("value") or "" - for entry in parsed_email.get("headers") or [] - if isinstance(entry, dict) and (entry.get("name") or "").lower() == target + entry.get("value") if isinstance(entry.get("value"), str) else "" + for entry in _header_entries(parsed_email) + if _entry_name_matches(entry, target) ] @@ -149,8 +221,7 @@ def has_header(parsed_email: dict[str, Any], name: str) -> bool: case-insensitively.""" target = name.lower() return any( - isinstance(entry, dict) and (entry.get("name") or "").lower() == target - for entry in parsed_email.get("headers") or [] + _entry_name_matches(entry, target) for entry in _header_entries(parsed_email) ) @@ -179,10 +250,21 @@ def body_part_text(parsed_email: dict[str, Any], part: dict[str, Any]) -> str: # attachment anyway. return inline if isinstance(inline, str) else "" part_id = part.get("partId") - if not part_id: + # ``partId`` is a lookup key, so it has to be a string before we index + # with it: an unhashable value (a dict, a list) raises TypeError from + # ``.get`` rather than returning the documented default. + if not isinstance(part_id, str) or not part_id: return "" - bv = (parsed_email.get("bodyValues") or {}).get(part_id) or {} - return bv.get("value") or "" + if not isinstance(parsed_email, dict): + return "" + body_values = parsed_email.get("bodyValues") + if not isinstance(body_values, dict): + return "" + entry = body_values.get(part_id) + if not isinstance(entry, dict): + return "" + value = entry.get("value") + return value if isinstance(value, str) else "" def body_text_joined(parsed_email: dict[str, Any], key: str = "textBody") -> str: @@ -194,5 +276,9 @@ def body_text_joined(parsed_email: dict[str, Any], key: str = "textBody") -> str "all body text as one string" pattern (snippet extraction, search indexing, audit logging). """ + if not isinstance(parsed_email, dict): + return "" parts = parsed_email.get(key) or [] + if not isinstance(parts, list): + return "" return "".join(body_part_text(parsed_email, p) for p in parts) diff --git a/src/jmap-email/jmap_email/options.py b/src/jmap-email/jmap_email/options.py index 309d200a..8725d7ad 100644 --- a/src/jmap-email/jmap_email/options.py +++ b/src/jmap-email/jmap_email/options.py @@ -33,7 +33,12 @@ Or replace one cap on the default by ``dataclasses.replace``:: from dataclasses import dataclass -__all__ = ["DEFAULT_PARSE_OPTIONS", "ParseOptions"] +__all__ = [ + "DEFAULT_COMPOSE_OPTIONS", + "DEFAULT_PARSE_OPTIONS", + "ComposeOptions", + "ParseOptions", +] @dataclass(frozen=True, slots=True) @@ -42,7 +47,8 @@ class ParseOptions: Pass an instance to :func:`jmap_email.parse_email` or :func:`jmap_email.parse_addresses` via the ``options=`` keyword. - Excess input is silently truncated and a WARNING is logged. + Excess input is silently truncated and a WARNING is logged, except + for ``max_header_value_bytes``, which **rejects** the message. Attributes ---------- @@ -56,11 +62,19 @@ class ParseOptions: flat ``multipart/mixed`` inputs with millions of children. Sourced from Go's ``multipartmaxparts``. Default: 1000. max_header_value_bytes : int - Maximum byte-length of a single header value retained for - downstream processing. Values above this size are truncated - before the stdlib's ``_header_value_parser`` runs — guards - against the quadratic-time hot spots reported in gh-136063. - Sourced from Postfix's ``header_size_limit``. Default: 102 400. + Maximum octet-length of a single header value. A message + carrying a longer field is **rejected** — ``parse_email`` + returns ``None`` — rather than truncated: there is no generally + safe cut point for an arbitrary field, and a byte cut can + manufacture a value that was never sent (a shortened address + list re-parses to a different address). Also guards the + quadratic-time hot spots reported in gh-136063. + + RFC 5322 §2.2.3 puts no limit on a field — folding makes it + unbounded while every line stays legal — so this is local + policy, sourced from Postfix's ``header_size_limit``. Postfix + discards the excess; we refuse, because it is rewriting a header + where we assert identity from one. Default: 102 400. max_address_list_bytes : int Maximum byte-length of an address-list value handed to :func:`jmap_email.parse_addresses`. Cap protects against the @@ -95,3 +109,92 @@ class ParseOptions: DEFAULT_PARSE_OPTIONS = ParseOptions() + + +@dataclass(frozen=True, slots=True) +class ComposeOptions: + """Per-call compose policy: what the composer is permitted to emit. + + Pass an instance to :func:`jmap_email.compose_email` via the + ``options=`` keyword. Unlike :class:`ParseOptions`, which is mostly + resource caps against hostile input, these are output-correctness + choices — the composer is strict by design and each of these names a + place where "strict" is genuinely deployment-dependent rather than + universal. + + Per-*message* values (``in_reply_to``, ``prepend_headers``) stay + keyword arguments on ``compose_email``: they change on every call, + where everything here is a property of the call site and is set once. + + Two of these name ESMTP capabilities of the hop the bytes are + headed for. The library does not discover those — the caller does, + and how is the caller's business: a fixed relay whose configuration + you own, an EHLO response you composed *after* reading, or two + variants composed up front so the delivery loop can fall back when + the server answers ``SMTPNotSupportedError``. See the README for + that last pattern. + + Attributes + ---------- + emit_bcc : bool + When False, the ``Bcc:`` header is silently dropped — the entire + point of Bcc is that it must NOT be transmitted to recipients. + Set True only for archive reconstruction (e.g. PST import, where + the Bcc list was already in the source file). Default: False. + idna_encode_domains : bool + When True, an addr-spec whose *domain* is non-ASCII is IDNA + encoded to its A-label form on the wire + (``contact@exemplé.fr`` → ``contact@xn--exempl-gva.fr``). + + Unlike the two flags below, this one names no ESMTP capability: + an A-label is a plain ASCII domain, valid at every hop with no + extension involved, and it is the only form the MX lookup can + use. It is off by default because the composer does not rewrite + an address the caller gave it unless asked — not because the + wire might refuse it. Default: False. + + It governs the domain only. A non-ASCII **local part** needs + ``allow_smtputf8``: punycode is a DNS algorithm and a local part is not + a DNS label, so there is nothing to encode it to. + allow_8bit : bool + When True, non-ASCII bodies are emitted as raw 8-bit octets + instead of being promoted to quoted-printable or base64. Requires + the next hop to advertise 8BITMIME (RFC 6152); a relay without it + either refuses the message or mangles it. Worth roughly the 33% + base64 overhead on non-ASCII bodies, and leaves the message + readable on the wire. Default: False. + allow_smtputf8 : bool + When True, headers are emitted as UTF-8 (RFC 6532) rather than + RFC 2047 encoded-words, and a non-ASCII **local part** is + permitted instead of raising. Requires the next hop to advertise + SMTPUTF8 (RFC 6531) *and* the caller to put the ``SMTPUTF8`` + parameter on ``MAIL FROM``. + + There is no downgrade: RFC 6530 dropped the mechanism RFC 5504 + had specified, so a message whose mailbox names require UTF-8 + cannot be rewritten into an ASCII equivalent — if the hop lacks + the extension, the message bounces. Support is also not + transitive; a relay that accepts your transaction may forward to + one that cannot. Compose the ASCII variant too if you need a + fallback. Default: False. + """ + + emit_bcc: bool = False + idna_encode_domains: bool = False + allow_8bit: bool = False + allow_smtputf8: bool = False + + @property + def emits_8bit(self) -> bool: + """True when the output may contain raw 8-bit octets. + + ``allow_smtputf8`` implies it: RFC 6532 headers are UTF-8, which is + 8-bit by construction, and RFC 6531 §3.1 requires any server + advertising SMTPUTF8 to support 8BITMIME as well. Encoding the + *body* down to 7-bit while emitting 8-bit *headers* would buy + nothing. + """ + return self.allow_8bit or self.allow_smtputf8 + + +DEFAULT_COMPOSE_OPTIONS = ComposeOptions() diff --git a/src/jmap-email/jmap_email/parser.py b/src/jmap-email/jmap_email/parser.py index 4ec7bddd..284ad4c1 100644 --- a/src/jmap-email/jmap_email/parser.py +++ b/src/jmap-email/jmap_email/parser.py @@ -18,64 +18,45 @@ must be guarded here, not assumed upstream. """ import base64 -import email import hashlib import logging import re import unicodedata -from collections import defaultdict +from collections import Counter, defaultdict from datetime import datetime from datetime import timezone as dt_timezone from email import policy as email_policy from email.errors import HeaderParseError, MessageError +from email.feedparser import ( + BufferedSubFile, # ty: ignore[unresolved-import] + BytesFeedParser, +) from email.header import decode_header as _stdlib_decode_header from email.message import Message -from email.utils import getaddresses, parsedate_to_datetime -from ntpath import basename as nt_basename -from posixpath import basename as posix_basename +from email.utils import ( + collapse_rfc2231_value, + getaddresses, + parsedate_to_datetime, +) from typing import Any, cast +from .addresses import is_valid_addr_spec +from .filenames import sanitize_filename from .options import DEFAULT_PARSE_OPTIONS, ParseOptions from .preview import preview_text from .types import EmailAddress, EmailBodyPart, JmapEmail -# Resource caps — all chosen to match or exceed the equivalents in -# battle-tested mail servers. Real-world legitimate messages are well -# below these caps; adversarial inputs that exceed them are silently -# truncated or rejected per Postel's law. +# Resource caps and their provenance live on +# :class:`jmap_email.options.ParseOptions`; per-call overrides go through +# the ``options=`` keyword. Read ``DEFAULT_PARSE_OPTIONS`` where no +# per-call bundle is in scope. # -# - Postfix ``mime_nesting_limit`` defaults to 100. Above this depth, -# Python's ~1000-frame recursion limit becomes reachable through a -# crafted ``multipart/mixed`` cascade. -# - Go's stdlib (after CVE-2022-41725 / CVE-2023-24536 / CVE-2023-45290) -# caps multipart parts at 1000 per message via the -# ``multipartmaxparts`` GODEBUG. Python's ``email`` package has no -# equivalent; we enforce our own here. -# - Postfix ``header_size_limit`` is 102_400 bytes — the de-facto -# ceiling we copy. Anything larger is truncated before decoding; -# ``email`` package decoders are linear in input size, so a 10 MB -# ``X-Foo`` header still works but burns wall-clock. -# - Postfix ``header_address_token_limit`` is 10_240 tokens. We cap -# the *byte length* of an address-list header instead (100 KB, -# roughly 5_000 typical addresses) — getaddresses is O(n) but the -# per-tuple allocations stack up on huge inputs (Dovecot -# CVE-2024-23184 was the same anti-pattern). -# ``message/rfc822`` nesting is implicitly bounded: we treat -# ``message/*`` parts as opaque attachments (we don't recurse into -# them in ``_parse_body_structure``), so a hostile chain of nested -# forwards can only hurt us via stdlib's ``Message.as_bytes()`` when -# we serialize the wrapped sub-message in ``_decoded_part_body``. -# That call catches ``RecursionError`` directly. -# Module-level mirrors of the default resource caps. Authoritative -# values live on :class:`jmap_email.options.ParseOptions`; per-call -# overrides go through the ``options=`` keyword on :func:`parse_email` -# / :func:`parse_addresses`. Module-level reassignment is not a -# supported tuning mechanism — it would race across threads and leak -# across unrelated callers in the same process. -MAX_MIME_NESTING_DEPTH = DEFAULT_PARSE_OPTIONS.max_mime_nesting_depth -MAX_MIME_PARTS = DEFAULT_PARSE_OPTIONS.max_mime_parts -MAX_HEADER_VALUE_BYTES = DEFAULT_PARSE_OPTIONS.max_header_value_bytes -MAX_ADDRESS_LIST_BYTES = DEFAULT_PARSE_OPTIONS.max_address_list_bytes +# ``message/rfc822`` nesting is implicitly bounded and so has no cap: we +# treat ``message/*`` parts as opaque attachments (no recursion in +# ``_parse_body_structure``), so a hostile chain of nested forwards can +# only hurt us via stdlib's ``Message.as_bytes()`` when we serialize the +# wrapped sub-message in ``_decoded_part_body``. That call catches +# ``RecursionError`` directly. # Characters stripped from decoded display-names before they are # surfaced. Header-injection vector: a downstream consumer that re- @@ -103,6 +84,101 @@ logger = logging.getLogger(__name__) _PARSE_POLICY = email_policy.compat32 +# ─── Nesting-depth cost ─── +# +# ``BufferedSubFile.readline`` tests every body line against every +# ancestor predicate, which makes the stdlib parse O(depth × lines) — +# enough for one legal-but-deep message to occupy a worker. +# ``max_mime_nesting_depth`` could not help: it is applied during the +# body-tree walk, by which point the parse has already been paid for. +# +# RFC 2046 §5.1.1 requires a *boundary* delimiter to begin with ``--``, +# so while every active predicate is a boundary matcher the scan can be +# skipped for any other line. That makes cost flat in depth with +# identical output. The stack is not boundary-only, though — see the +# class — so this is not an optimisation CPython simply missed. + + +class _FastSubFile(BufferedSubFile): + """``BufferedSubFile`` without the per-ancestor scan on body lines. + + The scan may only be skipped while every *active* predicate requires + a ``--`` prefix. ``_eofstack`` is not boundary-only: inside a + ``message/delivery-status`` the parser pushes ``NLCRE.match``, for + which a *blank* line is the false EOF. Skipping that merges the DSN's + per-message and per-recipient blocks into one and turns + ``Final-Recipient`` / ``Status`` into an unparsed payload. + + So the predicates are tracked as they are pushed, and anything not + recognised as a boundary matcher disables the shortcut for as long + as it is on the stack — unrecognised means slow, never wrong. + """ + + def __init__(self): + super().__init__() + self._delimiter_only = True + + def _refresh(self): + self._delimiter_only = all( + getattr(pred, "__name__", "") == "boundarymatch" for pred in self._eofstack + ) + + def push_eof_matcher(self, pred): + super().push_eof_matcher(pred) + self._refresh() + + def pop_eof_matcher(self): + pred = super().pop_eof_matcher() + self._refresh() + return pred + + def readline(self): + lines = self._lines + if lines and self._eofstack and self._delimiter_only and lines[0][:2] != "--": + return lines.popleft() + return super().readline() + + +def _message_from_bytes(raw: bytes) -> Message: + """``email.message_from_bytes`` with the faster input buffer. + + Substituting ``_input`` is the whole integration: the parser is + otherwise the stdlib's, fed in one shot, and the result is + byte-identical. ``_probe_fast_subfile`` verifies the substitution + still takes effect rather than assuming it. + """ + feed_parser = BytesFeedParser(policy=_PARSE_POLICY) + feed_parser._input = _FastSubFile() # ty: ignore[unresolved-attribute] # noqa: SLF001 + feed_parser.feed(raw) + return feed_parser.close() + + +def _probe_fast_subfile() -> bool: + """Verify the substituted buffer is actually used by the parser. + + ``_input`` is private, so this pins the coupling to a startup check + instead of a silent no-op. ``tests/test_parser.py`` asserts the + result, so a Python upgrade that moves it fails CI. + """ + try: + probe = BytesFeedParser(policy=_PARSE_POLICY) + probe._input = _FastSubFile() # ty: ignore[unresolved-attribute] # noqa: SLF001 + probe.feed(b'Content-Type: multipart/mixed; boundary="B"\n\n--B\n\nx\n--B--\n') + used = isinstance(probe._input, _FastSubFile) # ty: ignore[unresolved-attribute] # noqa: SLF001 + return used and probe.close().is_multipart() + except Exception: # pylint: disable=broad-exception-caught + return False + + +_FAST_SUBFILE_SUPPORTED = _probe_fast_subfile() + +if not _FAST_SUBFILE_SUPPORTED: # pragma: no cover + logger.error( + "The faster MIME input buffer is not taking effect on this Python; " + "deeply nested messages will parse slowly." + ) + + def _strip_nul_bytes(text: str) -> str: """Strip NUL bytes from text. @@ -144,13 +220,28 @@ def _repair_surrogate_escaped(text: str) -> str: def decode_rfc2047_header(header_text: str) -> str: """Decode RFC 2047 encoded-words in a header value to a single string. - Wraps :func:`email.header.decode_header` (stdlib) with three + Wraps :func:`email.header.decode_header` (stdlib) with four additional guarantees the bare stdlib helper doesn't give: a single string return type (not a list of fragments); recovery from ``HeaderParseError`` on malformed base64 inside an encoded- word so a single bad ``=?…?b?…?=`` doesn't torpedo the rest of - the parse; and surrogate-escape repair of raw 8-bit bytes left - by the ``compat32`` policy. + the parse; surrogate-escape repair of raw 8-bit bytes left + by the ``compat32`` policy; and a bound on the input. + + The bound is required: ``decode_header`` pops from the front of a + list, making it O(n²) in the number of encoded-words. ``parse_email`` + rejects an over-long header value outright, but that check runs over + ``message.raw_items()`` — the top-level fields only. A MIME *part* + header is never seen by it, so an attachment filename off a + sub-part's ``Content-Disposition`` reaches here unbounded and this is + the only thing standing in front of it. + + A work bound, not a policy limit: it borrows the magnitude of + ``max_header_value_bytes`` but counts characters, which is what the + fragment count — and so the cost — actually tracks. Deliberately not + a ``ParseOptions`` knob: the call site that needs it + (``_get_part_info``) has no options to thread, and a parameter no + internal caller can honour is worse than none. Folding CRLF+WSP is unfolded to single spaces; other internal whitespace runs are preserved. @@ -159,6 +250,12 @@ def decode_rfc2047_header(header_text: str) -> str: return "" header_text_str = str(header_text) + cap = DEFAULT_PARSE_OPTIONS.max_header_value_bytes + if len(header_text_str) > cap: + logger.warning( + "decode_rfc2047_header: input exceeds %d characters; truncating", cap + ) + header_text_str = header_text_str[:cap] # Stdlib's ``email.header.decode_header`` returns a list of # ``(decoded_string, charset)`` pairs (charset is ``None`` when the # fragment was not encoded). It raises ``HeaderParseError`` on @@ -387,7 +484,67 @@ def _is_plausible_addr(addr: str) -> bool: # inserts, log lines, JSON serialisers — never see them). if any(c in addr for c in ("\r", "\n", "\t", "\x00")): return False - return True + # 3. **Not one mailbox.** ``a@b.co, c@d.co`` and ``a b@c.co`` both + # carry an ``@`` and no control characters, but neither is a + # single addr-spec: the comma is the mailbox-list separator and + # the space separates a display name from an angle-addr, so a + # consumer re-emitting either into a header gains a recipient it + # never validated. Same shape as MimeKit's CVE-2026-30227. + # Delegated to the composer's predicate so the parse and compose + # sides cannot drift apart on what counts as an address. + return is_valid_addr_spec(addr) + + +# An angle-addr sitting inside a display name means the split went wrong. +# The local half excludes ``@`` so the split is unique; both halves +# matching it is quadratic on a ``<`` + run of ``@``. +_ANGLE_ADDR_RE = re.compile(r"<[^<>@]*@[^<>]*>") + + +def _has_unclosed_comment(value: str) -> bool: + """True when *value* opens an RFC 5322 comment it never closes. + + Parens inside a quoted-string are literal text and a quoted-pair + escapes whatever follows it, so both are skipped. + """ + depth = 0 + in_quotes = False + index = 0 + while index < len(value): + char = value[index] + if char == "\\": + index += 2 + continue + if char == '"': + in_quotes = not in_quotes + elif not in_quotes: + if char == "(": + depth += 1 + elif char == ")" and depth: + depth -= 1 + index += 1 + return depth > 0 + + +def _comment_ate_the_angle_addr(source: str, name: str) -> bool: + """True when an unclosed comment swallowed the real addr-spec. + + ``getaddresses(strict=False)`` treats an unclosed ``(`` as a comment + running to end of input. In ``victim@bank.com( `` + that comment eats the angle-addr, so the splitter reports a single + tuple whose *address* is the text the sender typed in display-name + position and whose *name* holds the mailbox they actually own. + + That is CVE-2023-27043 reached by another route: + :func:`_pick_best_address` defends the multi-tuple form by taking + the last plausible tuple, but here the bogus tuple is the only one + there is. A consumer keying allow/deny, DMARC alignment or contact + identity off the parsed address would attribute the message to an + address the sender merely named. Both conditions are required so + that a quoted display name legitimately containing an angle-addr + (``"Bob " ``) still parses. + """ + return bool(_ANGLE_ADDR_RE.search(name)) and _has_unclosed_comment(source) def _pick_best_address(parsed) -> tuple[str, str] | None: @@ -497,9 +654,51 @@ def parse_address( return ("", address_str.strip()) if lenient else ("", "") name, addr = _clean_address_pair(*best) + if _comment_ate_the_angle_addr(address_str, name): + return ("", address_str.strip()) if lenient else ("", "") return name, addr +def _last_separator_index(value: str, limit: int) -> int: + """Index of the last mailbox-list ``,`` at or before *limit*, or -1. + + Only a top-level comma separates mailboxes: one inside a + quoted-string, a domain-literal or a comment is literal text. + Cutting at those leaves a fragment that re-parses into an address + nobody sent — ``a@[1,2]`` cut at its comma yields ``a@[1]``. + """ + in_quote = False + in_literal = False + comment_depth = 0 + last = -1 + index = 0 + end = min(len(value), limit) + while index < end: + char = value[index] + if char == "\\" and (in_quote or comment_depth): + index += 2 # quoted-pair escapes whatever follows + continue + if in_quote: + in_quote = char != '"' + elif in_literal: + in_literal = char != "]" + elif comment_depth: + if char == "(": + comment_depth += 1 + elif char == ")": + comment_depth -= 1 + elif char == '"': + in_quote = True + elif char == "[": + in_literal = True + elif char == "(": + comment_depth = 1 + elif char == ",": + last = index + index += 1 + return last + + def parse_addresses( addresses_str: str, *, @@ -519,6 +718,11 @@ def parse_addresses( address-tuple recovery, never "the whole header was garbage so return it as a single fake address." + A value over ``max_address_list_bytes`` is cut back to a mailbox + separator, so entries past the cut are missing from the result. Use + :func:`_parse_addresses_ex` when you need to know that happened; + ``parse_email`` does, and reports ``AddressListTruncatedDefect``. + Args: addresses_str: Comma-separated string of email addresses. options: Per-call resource caps. See :class:`ParseOptions`. Pass @@ -529,18 +733,40 @@ def parse_addresses( List of tuples, each containing (display_name, email_address). Entries that fail the addr-spec shape check are omitted. """ + return _parse_addresses_ex(addresses_str, options)[0] + + +def _parse_addresses_ex( + addresses_str: str, options: ParseOptions +) -> tuple[list[tuple[str, str]], bool]: + """:func:`parse_addresses`, also reporting whether the input was cut. + + Truncation is a property of the parse, so it is decided here rather + than re-derived by the caller — two copies of the cut condition would + drift the moment one changed. + """ if not addresses_str: - return [] + return [], False # Defensive byte cap. ``getaddresses`` is O(n) but a 50 MB # ``To:`` would allocate millions of tuples (see Dovecot # CVE-2024-23184 — same anti-pattern in C). The default 100 KB cap # holds ~5_000 typical addresses; well above any legitimate # mailing-list expansion that lands in a single header. + # Cut back to the last mailbox separator so the result is always a + # subset of what an uncapped parse returns. Slicing at the byte bound + # instead lands mid-token and can *manufacture* an address: pad a + # header so the cut falls after ``, ceo@corp.example`` in + # ``…, ceo@corp.example-junk@attacker.test`` and the list parses to + # exactly ``ceo@corp.example``, which nobody sent. Same shape as + # Postfix's ``header_address_token_limit``, which discards excess + # *tokens* rather than bytes. cap = options.max_address_list_bytes - if len(addresses_str) > cap: + truncated = len(addresses_str) > cap + if truncated: logger.warning("Address-list header exceeds %d bytes; truncating", cap) - addresses_str = addresses_str[:cap] + separator = _last_separator_index(addresses_str, cap) + addresses_str = addresses_str[:separator] if separator >= 0 else "" # Repair raw 8-bit (surrogate-escaped) bytes. See # ``parse_address`` for why we deliberately stop short of @@ -552,7 +778,7 @@ def parse_addresses( if _contains_group_syntax(addresses_str): addresses_str = _remove_group_syntax(addresses_str) if not addresses_str: - return [] # Empty group like "undisclosed-recipients:;" + return [], truncated # Empty group like "undisclosed-recipients:;" try: parsed = getaddresses([addresses_str], strict=False) @@ -563,7 +789,7 @@ def parse_addresses( # limit. Degrade to empty rather than letting the error # propagate. logger.warning("RecursionError in getaddresses; returning empty list") - return [] + return [], truncated result: list[tuple[str, str]] = [] for raw_name, raw_addr in parsed: @@ -573,8 +799,10 @@ def parse_addresses( if not _is_plausible_addr(raw_addr): continue name, addr = _clean_address_pair(raw_name, raw_addr) + if _comment_ate_the_angle_addr(addresses_str, name): + continue result.append((name, addr)) - return result + return result, truncated def parse_date(date_str: str) -> datetime | None: @@ -598,100 +826,317 @@ def parse_date(date_str: str) -> datetime | None: return None -def _infer_filename_from_content_type(content_type: str) -> str: - """ - Infer a filename with extension from a MIME content type. - Uses the most commonly used file extensions for each MIME type. +# Transfer encodings RFC 2045 §6.1 defines. Anything else on the wire is +# a value some parser in the chain will interpret and another won't. +_KNOWN_TRANSFER_ENCODINGS = frozenset( + {"7bit", "8bit", "binary", "base64", "quoted-printable"} +) - Args: - content_type: MIME type string (e.g., "image/png", "application/pdf") - Returns: - Filename with appropriate extension (e.g., "unnamed.png", "unnamed.pdf") - """ - extension_map = { - "text/plain": ".txt", - "text/html": ".html", - "text/csv": ".csv", - "application/pdf": ".pdf", - "image/jpeg": ".jpg", - "image/png": ".png", - "image/gif": ".gif", - "application/json": ".json", - "application/xml": ".xml", - "application/zip": ".zip", +# RFC 5322 §3.6: each of these may appear at most once. Real senders +# emit duplicates anyway, and which copy a reader sees is parser-defined. +_RFC5322_SINGLETON_HEADERS = frozenset( + { + "date", + "sender", + "reply-to", + "to", + "cc", + "bcc", + "message-id", + "in-reply-to", + "references", + "subject", } - ext = extension_map.get(content_type, "") - return f"unnamed{ext}" +) -def _sanitize_filename(filename: str, max_length: int = 255) -> str: - """Sanitize an attachment filename, preserving the extension when truncating.""" +def _flatten_param(value: Any) -> str | None: + """Reduce a ``get_param`` result to a plain string. - filename = nt_basename(posix_basename(filename)) - - filename = filename.strip('"/.\\') - - # Remove null bytes and control characters - filename = re.sub(r"[\x00-\x1f\x7f]", "", filename) - - # Remove dangerous characters - filename = re.sub(r'[<>:"|?*\\/]', "_", filename) - - # Truncate while preserving extension - if len(filename) > max_length: - # Find the last dot for extension (but not at the start like .gitignore) - last_dot = filename.rfind(".") - if last_dot > 0: - name = filename[:last_dot] - ext = filename[last_dot:] - # Only preserve extension if it's reasonable length (up to 10 chars including dot) - if len(ext) <= 10: - max_name_length = max_length - len(ext) - if max_name_length > 0: - return name[:max_name_length] + ext - return filename[:max_length] - - return filename - - -def _build_attachment_dict( - body: Any, - part_type: str, - filename: str, - disposition: str, - content_id: str | None, -) -> dict[str, Any]: + RFC 2231-encoded parameters come back as a ``(charset, lang, value)`` + tuple; everything else is already a string. """ - Helper function to build an attachment dictionary. - Converts body to bytes, computes SHA-256 hash, and constructs the attachment dict. + if value is None: + return None + if isinstance(value, tuple): + # ``(charset, lang, value)`` — the value is percent-decoded bytes + # carried in a latin-1 str, so it needs the charset applied. Doing + # ``value[2]`` alone turns "documenté.pdf" into "documenté.pdf". + try: + value = collapse_rfc2231_value(value) + except Exception: # pylint: disable=broad-exception-caught + value = value[-1] + text = str(value).strip() + return text or None - Args: - body: The part body (str or bytes) - part_type: MIME type of the part - filename: Name of the attachment file - disposition: Content-Disposition value ("attachment", "inline", etc.) - content_id: Content-ID if present - Returns: - Dictionary representing the attachment +def _preferred_filename(part: Message) -> str | None: + """Return the filename a spec-following recipient would use. + + RFC 6266 §4.3: when a ``Content-Disposition`` carries both + ``filename`` and the RFC 2231 extended ``filename*``, recipients + SHOULD pick ``filename*`` and ignore ``filename``. The stdlib's + ``get_filename`` returns whichever the parameter dict happened to + keep — in practice the plain one — so a sender can show + ``safe.txt`` to us and ``evil.exe`` to every client that follows the + RFC. Prefer the extended form, then fall back to the stdlib + (which also covers the ``Content-Type: name=`` case). """ - if isinstance(body, str): - body_bytes = body.encode("utf-8") - else: - body_bytes = body + try: + params = part.get_params(header="content-disposition") or [] + except Exception: # pylint: disable=broad-exception-caught + return part.get_filename() + for key, value in params: + if key == "filename" and isinstance(value, tuple): + return _flatten_param(value) + return part.get_filename() - content_hash = hashlib.sha256(body_bytes).hexdigest() - return { - "type": part_type, - "name": _sanitize_filename(filename) or "unnamed", - "size": len(body_bytes), - "disposition": disposition, - "cid": content_id, - "content": body_bytes, - "sha256": content_hash, +# Headers whose value steers how the body is carved up or named. A +# control character in one of these is a structural risk, not a display +# quirk, which is why the check is scoped to them rather than every header. +_MIME_RELEVANT_HEADERS = frozenset( + { + "content-type", + "content-disposition", + "content-transfer-encoding", + "content-id", } +) + + +def _iter_mime_header_values(part: Message): + """Yield ``(name, raw_value)`` for the MIME-structural headers.""" + try: + items = list(part.items()) + except Exception: # pylint: disable=broad-exception-caught + return + for name, value in items: + if str(name).lower() in _MIME_RELEVANT_HEADERS: + yield str(name), str(value) + + +# Content types whose payload this parser reports as one opaque +# attachment and does not look inside. ``message/rfc822`` is deliberately +# absent: it fires on every forwarded message, so it would be noise +# rather than signal (the README says as much). These two are not +# ordinary mail — +# +# message/partial RFC 2046 §5.2.2 splits one message across +# several, so the payload only exists after a +# reassembly a per-message scanner never does. +# message/external-body the content is fetched from elsewhere and is +# not in this message at all. +# +# — and both are long-standing ways to put something in front of a reader +# that no content scanner ever saw. +_OPAQUE_MESSAGE_DEFECTS = { + "message/partial": "PartialMessageDefect", + "message/external-body": "ExternalBodyDefect", +} + + +def _has_fold_inside_quotes(raw_value: str) -> bool: + """True when a header folds *inside* a quoted parameter value. + + Folding between parameters is ordinary. Folding inside the quotes is + where readers diverge: unfolding per RFC 5322 §2.2.3 removes the CRLF + and keeps the WSP, so ``filename="pay load.exe"`` is + ``pay load.exe`` here — but a parser that drops the whole fold reads + ``payload.exe`` and one that truncates at the CR reads ``pay``. Three + names for one attachment, which is the point of doing it. + """ + in_quotes = False + index = 0 + while index < len(raw_value): + char = raw_value[index] + if char == "\\" and in_quotes: + index += 2 + continue + if char == '"': + in_quotes = not in_quotes + elif in_quotes and char in "\r\n": + return True + index += 1 + return False + + +def _collect_message_ambiguity_defects(message: Message, defects: list[str]) -> None: + """Root-level ambiguities: duplicated singleton headers, missing + ``MIME-Version``. + + Kept separate from the per-part walk because these are properties of + the message as a whole — a nested ``message/rfc822`` legitimately + carries its own ``From``, and flagging that would be noise. + """ + try: + names = [str(k).lower() for k in message.keys()] + except Exception: # pylint: disable=broad-exception-caught + return + counts = Counter(names) + + # Called out separately from the rest: a second ``From`` is how a + # sender shows one identity to the filter that authenticated the + # message and another to the human who reads it (CERT VU#517845; + # "Weak Links in Authentication Chains", USENIX 2020). + if counts.get("from", 0) > 1: + defects.append("DuplicateFromDefect") + if any(counts.get(h, 0) > 1 for h in _RFC5322_SINGLETON_HEADERS): + defects.append("DuplicateScalarHeaderDefect") + + # MIME syntax with no MIME-Version to license it. We parse it; a + # strict receiver treats the body as flat text and never sees the + # parts — including their attachments. + if not counts.get("mime-version"): + try: + is_mime = message.get_content_maintype() == "multipart" or bool( + message.get("content-transfer-encoding") + ) + except Exception: # pylint: disable=broad-exception-caught + return + if is_mime: + defects.append("MissingMimeVersionDefect") + + +def _collect_ambiguity_defects(part: Message, defects: list[str]) -> None: + """Record MIME constructs that different parsers resolve differently. + + None of these stop us producing output — we resolve each the way the + stdlib does — but each is a point where a spam filter, a virus + scanner and a mail client can legitimately disagree about what the + message *contains*. Research on MIME parser differentials ("Email + Smuggling with Differential Fuzzing of MIME Parsers", Andarzian, + Meyers & Poll, 2025) demonstrates payloads smuggled past filters on + exactly these constructs, so a consumer running a scanning or + quarantine policy needs to see them rather than receive a confident + parse of one of the possible readings. + + They are surfaced as ``_ext.defects`` entries alongside the stdlib + ones — a signal, not a verdict. + """ + try: + content_types = part.get_all("content-type") or [] + encodings = part.get_all("content-transfer-encoding") or [] + except Exception: # pylint: disable=broad-exception-caught + # A sufficiently damaged header block can trip the stdlib's own + # accessors; the absence of a defect flag must never be the thing + # that aborts a parse. + return + + # Duplicate structural headers: we take the first, and so does the + # stdlib. Clients have been observed honouring the *second*, which + # changes the media type — and with it whether the body is one blob + # of text or a multipart tree whose attachments we never extract. + if len(content_types) > 1: + defects.append("DuplicateContentTypeDefect") + if len(encodings) > 1: + defects.append("DuplicateTransferEncodingDefect") + + # An encoding token outside RFC 2045 §6.1. We leave the body + # undecoded; lenient clients guess (base64 for "bas64", or + # quoted-printable whenever the data contains "="), which reveals + # content a scanner reading our output never sees. This also catches + # the mangled-header case ``Content-Transfer-Encoding:: base64``, + # where the value arrives as ``": base64"``. + if encodings: + token = str(encodings[0]).strip().lower() + if token and token not in _KNOWN_TRANSFER_ENCODINGS: + defects.append("UnrecognizedTransferEncodingDefect") + + # RFC 2046 §5.1 says the preamble and epilogue are to be ignored, and + # we ignore both. Mail clients have been observed rendering preamble + # text as the message's first line, so it reaches the reader and + # nothing downstream of this parser; the epilogue is the same gap at + # the other end of the body. + preamble = getattr(part, "preamble", None) + if isinstance(preamble, str) and preamble.strip(): + defects.append("NonEmptyPreambleDefect") + epilogue = getattr(part, "epilogue", None) + if isinstance(epilogue, str) and epilogue.strip(): + defects.append("NonEmptyEpilogueDefect") + + # Two boundaries declared on one part: whichever we honour, the other + # delimits parts we never see. (Inbox Invasion, CCS '24.) + try: + ct_params = part.get_params() or [] + cd_params = part.get_params(header="content-disposition") or [] + except Exception: # pylint: disable=broad-exception-caught + # A damaged header block can trip the stdlib accessors. Carry on + # with no parameters rather than returning: the checks below read + # other sources, and abandoning them would mean the most damaged + # messages — the ones most worth flagging — get the fewest markers. + ct_params, cd_params = [], [] + if sum(1 for k, _ in ct_params if k == "boundary") > 1: + defects.append("DuplicateBoundaryParameterDefect") + + # A multipart whose boundary is absent or empty has no agreed way to + # split: some parsers give up and treat the body as one flat part, + # others resynchronise on the next ``--`` line they find. + try: + is_multipart = part.get_content_maintype() == "multipart" + except Exception: # pylint: disable=broad-exception-caught + is_multipart = False + if is_multipart: + try: + boundary = _flatten_param(part.get_param("boundary")) + except Exception: # pylint: disable=broad-exception-caught + # Unreadable is not better than absent: either way there is no + # boundary anyone can agree on. + boundary = None + if not boundary: + defects.append("EmptyBoundaryDefect") + + # RFC 2047 encoded-words are not permitted in MIME parameters — RFC + # 2231 is the mechanism for non-ASCII there. Senders use them anyway, + # and receivers split: some decode, some show the raw ``=?…?=``. We + # decode, so an attachment can carry one name past a scanner that + # doesn't and a different one to the user. + for _key, value in list(ct_params) + list(cd_params): + flat = _flatten_param(value) or "" + if "=?" in flat and "?=" in flat: + defects.append("EncodedWordInParameterDefect") + break + + # NUL and other control characters in a MIME-relevant header value. + # Parsers disagree on all three available readings — strip them (as we + # do), truncate at the first one, or keep them — so the same + # ``report.pdf\x00.exe`` is three different filenames depending on who + # is looking. + for _name, raw_value in _iter_mime_header_values(part): + if any(ord(c) < 0x20 and c not in "\t\r\n" or c == "\x7f" for c in raw_value): + defects.append("ControlCharInHeaderDefect") + break + + # A fold inside a quoted parameter value. Legal, and read three + # different ways — see ``_has_fold_inside_quotes``. + for _name, raw_value in _iter_mime_header_values(part): + if _has_fold_inside_quotes(raw_value): + defects.append("FoldInQuotedParameterDefect") + break + + # A part we hand over without looking inside. + try: + opaque = _OPAQUE_MESSAGE_DEFECTS.get(part.get_content_type()) + except Exception: # pylint: disable=broad-exception-caught + opaque = None + if opaque: + defects.append(opaque) + + # The part names itself twice and differently. A recipient picking + # ``Content-Type: name=`` — or honouring RFC 6266 §4.3's preference + # for ``filename*`` — saves the file under a name the other never + # saw, which is the whole point of a double extension. + names = { + flat + for flat in (_flatten_param(v) for k, v in cd_params if k == "filename") + if flat + } + ct_name = _flatten_param(dict(ct_params).get("name")) + if ct_name: + names.add(ct_name) + if len(names) > 1: + defects.append("ConflictingAttachmentNameDefect") def _is_inline_media_type(content_type: str) -> bool: @@ -797,7 +1242,7 @@ def _get_part_info(part: Message) -> dict[str, Any]: # through ``decode_rfc2047_header`` for the encoded-word case # where the value sits on a Content-Type ``name=`` parameter. filename: str | None = None - raw_filename = part.get_filename() + raw_filename = _preferred_filename(part) if raw_filename: filename = decode_rfc2047_header(str(raw_filename).strip()) @@ -963,21 +1408,20 @@ def _build_attachment_from_part_info( ) -> EmailBodyPart: """Build a JMAP ``EmailBodyPart`` for the attachments array.""" disposition = part_info["disposition"] or disposition_override - raw_filename = part_info["name"] # JMAP spec: ``name`` is ``String | null``. We don't substitute # ``"unnamed"`` here — if a downstream consumer wants a default, - # they can synthesize one. We also map an empty sanitized result - # to ``None`` so callers don't have to ``or "fallback"`` it. + # they can synthesize one. ``sanitize_filename`` returns ``None`` + # for a missing name and for one that sanitizes away to nothing, + # which is exactly the field's shape. body_bytes = part_info["body"] or b"" if isinstance(body_bytes, str): body_bytes = body_bytes.encode("utf-8") - sanitized_name = _sanitize_filename(raw_filename) if raw_filename else "" return { "partId": part_info["part_id"], "blobId": None, "type": part_info["type"], "size": len(body_bytes), - "name": sanitized_name or None, + "name": sanitize_filename(part_info["name"]), "charset": part_info.get("charset") or None, "disposition": disposition, "cid": part_info["content_id"], @@ -1456,17 +1900,6 @@ def _jmap_addresses(pairs: list[tuple[str, str]]) -> list[EmailAddress] | None: return [{"name": name or None, "email": addr} for name, addr in pairs] -def _jmap_single_address(name: str, addr: str) -> list[EmailAddress] | None: - """Convert a single ``(name, addr)`` to a single-element list (or None). - - JMAP requires address-list fields to always be lists; even a header - with one mailbox surfaces as a 1-element ``EmailAddress[]``. - """ - if not addr: - return None - return [{"name": name or None, "email": addr}] - - def _jmap_message_ids(raw_header_value: str) -> list[str] | None: """Split a Message-ID / In-Reply-To / References header into ``String[]``. @@ -1834,7 +2267,7 @@ def _parse_email( # raises on malformed input under this policy — recoverable # damage is recorded in ``message.defects`` and we walk the # structure best-effort. - message = email.message_from_bytes(raw_email_bytes, policy=_PARSE_POLICY) + message = _message_from_bytes(raw_email_bytes) if message is None: logger.warning( @@ -1873,19 +2306,35 @@ def _parse_email( for k, v in message.raw_items(): raw_value = _repair_surrogate_escaped(str(v)) - # Defensive byte cap on individual header values. Matches - # Postfix's ``header_size_limit``. Above this size the - # quadratic-time hot spots in ``_header_value_parser`` - # (gh-136063: ``get_phrase`` / ``_parseparam`` / etc.) - # start to hurt — truncating early keeps wall-clock - # bounded on adversarial input. - if len(raw_value) > options.max_header_value_bytes: + # Cap on individual header values, matching Postfix's + # ``header_size_limit``. RFC 5322 §2.2.3 puts no limit on a + # field ("may be indeterminately long" — folding makes it + # unbounded while every line stays legal), so this is local + # policy, and above it the quadratic hot spots in + # ``_header_value_parser`` (gh-136063) start to hurt. + # + # Rejected, not truncated. Postfix discards the excess, but + # it is rewriting a header, not asserting identity from one: + # a byte cut lands mid-token and can *manufacture* a value + # that was never sent. There is no generally safe cut point + # for an arbitrary field — a shortened ``Received`` still + # looks well-formed to the trust-scope logic — so a message + # carrying one is refused rather than silently reshaped. + # + # Measured in octets, not characters: the value has already + # been decoded from the wire, so a non-ASCII field is up to + # 4x longer in bytes than in ``len()`` and a character count + # would let it past a cap named for bytes. + if ( + len(raw_value.encode("utf-8", "surrogateescape")) + > options.max_header_value_bytes + ): logger.warning( - "Header %s value exceeds %d bytes; truncating", + "parse_email: header %s exceeds %d bytes; returning None", k, options.max_header_value_bytes, ) - raw_value = raw_value[: options.max_header_value_bytes] + return None # NUL bytes in headers would either reach a downstream text # store (PostgreSQL rejects \x00 in TEXT) or smuggle past a # naive C-string parser. Strip before any decode + before @@ -1918,13 +2367,29 @@ def _parse_email( # Address fields. Use the raw (surrogate-repaired but NOT RFC # 2047-decoded) header values to defend against PortSwigger # "Splitting the Email Atom" smuggling. + truncated_address_lists: set[str] = set() + def _addrs(name: str) -> list[EmailAddress] | None: """Header-present → list (possibly empty after validation); header-absent → None. Spec mandates the null-vs-empty distinction (RFC 8621 §4.1.2.2).""" if name not in decoded_by_name: return None - return _jmap_addresses(parse_addresses(raw_addr_headers.get(name, ""))) + # ``options=`` must be threaded through: without it every + # address header is parsed under the module default, so + # ``max_address_list_bytes`` — the CVE-2024-23184 analogue, + # Dovecot's unbounded address-list allocation — was inert for + # anyone tuning it on ``parse_email``, which is the entry + # point that actually meets hostile mail. + addresses, truncated = _parse_addresses_ex( + raw_addr_headers.get(name, ""), options + ) + if truncated: + # Entries past the cut are dropped, so the reported list + # is shorter than the wire's — a recipient we never + # surface is one a consumer cannot act on. + truncated_address_lists.add(name) + return _jmap_addresses(addresses) jmap_from = _addrs("from") jmap_sender = _addrs("sender") @@ -1944,13 +2409,9 @@ def _parse_email( jmap_sent_at = _jmap_iso_date(parse_date(_first_value("date"))) # Defects collected from the stdlib parse + our recursive walks. + # Populated by the walks below and harvested after body parsing — + # see the collection site further down for why the order matters. defects: list[str] = [] - try: - for part in message.walk(): - for defect in getattr(part, "defects", ()) or (): - defects.append(type(defect).__name__) - except RecursionError: - defects.append("RecursionError") body_parts = _parse_message_content(message, options=options, defects=defects) text_body: list[EmailBodyPart] = body_parts["textBody"] @@ -1999,6 +2460,22 @@ def _parse_email( # ─── Extensions (project-specific) ─── if extensions: + # Harvested HERE, not before the body walk. The stdlib attaches + # several of its defects while *decoding* a payload rather than + # while parsing structure — ``InvalidBase64CharactersDefect``, + # ``InvalidBase64PaddingDefect``, ``InvalidBase64LengthDefect`` + # — so a collection that ran before ``_parse_message_content`` + # (and before ``_build_body_structure``) silently dropped every + # one of them. + try: + for part in message.walk(): + for defect in getattr(part, "defects", ()) or (): + defects.append(type(defect).__name__) + _collect_ambiguity_defects(part, defects) + except RecursionError: + defects.append("RecursionError") + _collect_message_ambiguity_defects(message, defects) + ext: dict[str, Any] = {"defects": defects} # Resent-* typed projection. RFC 8621 §4.1.3 names only the # 11 base convenience properties; Resent-* is a §4.1.2 @@ -2018,6 +2495,11 @@ def _parse_email( } if any(v is not None for v in resent.values()): ext["resent"] = resent + # After the ``resent`` projection: those ``_addrs`` calls feed + # ``truncated_address_lists`` too, and ``defects`` is aliased + # into ``ext``, so the append lands in ``_ext.defects``. + if truncated_address_lists: + defects.append("AddressListTruncatedDefect") result["_ext"] = ext return cast(JmapEmail, result) diff --git a/src/jmap-email/jmap_email/preview.py b/src/jmap-email/jmap_email/preview.py index 78754f3d..79684d8a 100644 --- a/src/jmap-email/jmap_email/preview.py +++ b/src/jmap-email/jmap_email/preview.py @@ -38,17 +38,30 @@ from html.parser import HTMLParser __all__ = ["preview_text"] # ── markdown / whitespace patterns (all run on the bounded head) ───────── +# +# Every line-anchored pattern below uses ``[ \t]`` rather than ``\s`` for its +# leading run. ``\s`` matches ``\n``, so under ``re.MULTILINE`` a ``^\s*`` +# rescans the entire following whitespace run from every line start — O(n²) on +# a body of alternating spaces and newlines. The head is bounded, but its size +# scales with ``max_preview_chars``, so a caller who raised that knob turned a +# 128 KiB body into tens of seconds of matching. Leading *horizontal* +# whitespace is what these constructs actually allow. + # Fence markers are dropped but the code content itself is kept. -_MD_CODE_FENCE_RE = re.compile(r"^\s*(```|~~~).*$", re.MULTILINE) -_MD_IMAGE_RE = re.compile(r"!\[([^\]]*)\]\([^)]*\)") -_MD_LINK_RE = re.compile(r"\[([^\]]*)\]\([^)]*\)") +_MD_CODE_FENCE_RE = re.compile(r"^[ \t]*(```|~~~).*$", re.MULTILINE) +# Every unbounded run below is bounded instead. ``[^\]]*`` on a body of +# ``[[[[…`` with no closing bracket rescans to end of head from each of the +# n start positions — quadratic. The head is bounded, but its size scales +# with ``max_chars``. +_MD_IMAGE_RE = re.compile(r"!\[([^\]]{0,500})\]\([^)]{0,2000}\)") +_MD_LINK_RE = re.compile(r"\[([^\]]{0,500})\]\([^)]{0,2000}\)") # ATX headers (``# Title``) and setext underlines (a line of only ``=``/``-`` # under a title — we drop the underline, the title on the line above stays). -_MD_HEADER_RE = re.compile(r"^\s{0,3}#{1,6}\s+", re.MULTILINE) +_MD_HEADER_RE = re.compile(r"^[ \t]{0,3}#{1,6}[ \t]+", re.MULTILINE) _MD_SETEXT_RE = re.compile(r"^[ \t]*[=-]{2,}[ \t]*$", re.MULTILINE) # A horizontal rule is a line of only -/*/_ (3+), possibly spaced. -_MD_HRULE_RE = re.compile(r"^\s*(?:[-*_]\s*){3,}$", re.MULTILINE) -_MD_LIST_RE = re.compile(r"^\s*(?:[-*+]|\d+[.)])\s+", re.MULTILINE) +_MD_HRULE_RE = re.compile(r"^[ \t]*(?:[-*_][ \t]*){3,}$", re.MULTILINE) +_MD_LIST_RE = re.compile(r"^[ \t]*(?:[-*+]|\d+[.)])[ \t]+", re.MULTILINE) # Hard breaks: a trailing backslash at end of line (tolerate CRLF # endings — ``$`` alone stops at the ``\r``). _MD_HARDBREAK_RE = re.compile(r"\\[ \t]*\r?$", re.MULTILINE) @@ -70,7 +83,12 @@ _MD_EMPHASIS_RE = re.compile(r"\*{1,3}|~~|`+|(?`` / ```` are not lost (schemes are # case-insensitive; without this the tag path drops them as markup). -_AUTOLINK = r"(?:https?://|mailto:)[^>\s]+|[^@>\s]+@[^@>\s]+\.[^@>\s]+" +# Bounded for the same reason. RFC 5321 caps a local-part at 64 octets +# and a domain at 255, so these cannot refuse a real address. +_AUTOLINK = ( + r"(?:https?://|mailto:)[^>\s]{1,2000}" + r"|[^@>\s]{1,256}@[^@>\s.]{1,256}(?:\.[^@>\s.]{1,256}){1,16}" +) _AUTOLINK_INNER_RE = re.compile(_AUTOLINK, re.IGNORECASE) _MD_AUTOLINK_RE = re.compile(rf"<({_AUTOLINK})>", re.IGNORECASE) diff --git a/src/jmap-email/pyproject.toml b/src/jmap-email/pyproject.toml index 8c7191a2..8c25bfdd 100644 --- a/src/jmap-email/pyproject.toml +++ b/src/jmap-email/pyproject.toml @@ -1,7 +1,7 @@ [project] name = "jmap-email" version = "0.2.0" -description = "A strict-JMAP RFC 8621 Email object library for Python 3.14+ with lenient RFC 5322 / MIME parsing and strict-by-design composition. Zero runtime dependencies." +description = "A strict-JMAP RFC 8621 Email object library for Python 3.14+ with lenient RFC 5322 / MIME parsing and strict-by-design composition. One runtime dependency: idna (UTS 46 domain encoding)." readme = "README.md" license = "MIT" authors = [{ name = "ANCT", email = "contact@suite.anct.gouv.fr" }] @@ -18,10 +18,20 @@ classifiers = [ "Topic :: Software Development :: Libraries :: Python Modules", ] -# Zero runtime dependencies. The whole point of this library is to be a -# clean stdlib-only wrapper. If you find yourself adding one, stop and -# read the README first. -dependencies = [] +# One deliberate runtime dependency, and only this one. The stdlib's +# ``idna`` codec is IDNA2003, whose nameprep *folds* what it does not +# refuse (``faß.de`` → ``fass.de``, a distinct registrable domain since +# 2010) — silently routing mail to the folded sibling. The ``idna`` +# package does UTS 46 non-transitional, what browsers and modern +# resolvers do. Floor 3.7: the CVE-2024-3651 fix (quadratic +# ``idna.encode`` on crafted input). Cap <4: what a domain encodes to +# is this library's observable behavior, so a major that may revise +# UTS 46 mappings must not arrive silently. Never ``==``: an exact pin +# here would hold every consumer's ``idna`` hostage to our release +# cadence — including for security fixes like the one the floor names. +# Exact pinning is the consuming app's job (its lockfile). If you find +# yourself adding a *second* dependency, stop and read the README first. +dependencies = ["idna>=3.7,<4"] [project.urls] Homepage = "https://github.com/suitenumerique/messages/tree/main/src/jmap-email" diff --git a/src/jmap-email/tests/test_address_fuzz.py b/src/jmap-email/tests/test_address_fuzz.py index 35e6cb0b..463b903e 100644 --- a/src/jmap-email/tests/test_address_fuzz.py +++ b/src/jmap-email/tests/test_address_fuzz.py @@ -8,8 +8,10 @@ Run with: pytest -m fuzz core/tests/mda/test_rfc5322_address_fuzz.py Or: make fuzz-back """ +import os + import pytest -from hypothesis import HealthCheck, Phase, given, settings +from hypothesis import HealthCheck, given, settings from hypothesis import strategies as st from jmap_email.parser import ( @@ -21,10 +23,16 @@ from jmap_email.parser import ( # Intensive fuzzing settings FUZZ_SETTINGS = { - "max_examples": 10000, + "max_examples": int(os.environ.get("FUZZ_EXAMPLES", "10000")), "deadline": None, # No time limit per example "suppress_health_check": [HealthCheck.too_slow, HealthCheck.data_too_large], - "phases": [Phase.generate, Phase.target], # Skip shrinking for speed + # Phases are Hypothesis's defaults on purpose. ``shrink`` and + # ``explain`` cost nothing on a green run — they only engage once a + # failure exists, which is exactly when you want a minimal example + # rather than the raw generated blob. ``reuse`` replays a stored + # failure until it is fixed, which is what makes an intermittent + # find reproducible; it needs ``.hypothesis`` to survive the + # container, so compose mounts it. } @@ -370,3 +378,63 @@ class TestAddressEdgeCasesFuzzing: assert isinstance(result, tuple) result = parse_addresses(chars) assert isinstance(result, list) + + +@pytest.mark.fuzz +class TestAddrSpecMailboxCount: + """The invariant behind :func:`is_valid_addr_spec`. + + The predicate's job is not "looks like an address" — it is "this is + **one** mailbox, safe to place in a header as it stands". So the + property to hold it to is arithmetic: put an accepted value next to + one other recipient in a mailbox-list, and a reader must count + exactly two. Anything that can end up quoting, or being quoted by, + its neighbour breaks that count, which is how an unterminated + quoted-string local-part hid a recipient before it was rejected. + """ + + @given( + interior=st.text( + alphabet=st.sampled_from('abc \\",;<>@.()[]:'), + max_size=12, + ) + ) + @settings(**FUZZ_SETTINGS) + def test_accepted_quoted_local_part_is_exactly_one_mailbox(self, interior): + from email.utils import getaddresses + + from jmap_email import is_valid_addr_spec + + addr = f'"{interior}"@e.co' + if not is_valid_addr_spec(addr): + return + pairs = getaddresses([f"{addr}, victim@x.co"]) + assert len(pairs) == 2, f"{addr!r} did not stay one mailbox: {pairs!r}" + assert pairs[-1][1] == "victim@x.co" + + @given( + interior=st.text( + alphabet=st.sampled_from("ab1.:,;<>@()[]\\\"' "), + max_size=12, + ) + ) + @settings(**FUZZ_SETTINGS) + def test_accepted_domain_literal_is_exactly_one_mailbox(self, interior): + """Same arithmetic, for the other bracketed form. + + A comma or a paren inside ``[...]`` is legal dtext, but a reader + that does not track literal brackets cuts the list or opens a + comment there — which is how ``x@[a,b]`` next to a second + recipient collapsed to zero recovered mailboxes before those + characters were rejected. + """ + from email.utils import getaddresses + + from jmap_email import is_valid_addr_spec + + addr = f"a@[{interior}]" + if not is_valid_addr_spec(addr): + return + pairs = getaddresses([f"{addr}, victim@x.co"]) + assert len(pairs) == 2, f"{addr!r} did not stay one mailbox: {pairs!r}" + assert pairs[-1][1] == "victim@x.co" diff --git a/src/jmap-email/tests/test_ambiguity_defects.py b/src/jmap-email/tests/test_ambiguity_defects.py new file mode 100644 index 00000000..c0451696 --- /dev/null +++ b/src/jmap-email/tests/test_ambiguity_defects.py @@ -0,0 +1,550 @@ +"""Tests for the parser-ambiguity defects surfaced in ``_ext.defects``. + +Each construct below is one a spam filter, a virus scanner and a mail +client can legitimately resolve differently. We resolve them the way the +stdlib does; these markers exist so a consumer running a quarantine or +scanning policy can see that a choice was made at all. + +Motivated by "Email Smuggling with Differential Fuzzing of MIME Parsers" +(Andarzian, Meyers & Poll, 2025), which demonstrates payloads smuggled +past filters on exactly these constructs. +""" + +import pytest + +from jmap_email import compose_email, parse_email + + +def defects_of(raw: bytes) -> list[str]: + parsed = parse_email(raw, extensions=True) + assert parsed is not None + return (parsed.get("_ext") or {}).get("defects") or [] + + +CLEAN = ( + b"From: a@b.co\n" + b"Subject: t\n" + b"MIME-Version: 1.0\n" + b'Content-Type: multipart/mixed; boundary="----=_x"\n' + b"\n" + b"------=_x\n" + b"Content-Type: text/plain\n" + b"Content-Transfer-Encoding: 7bit\n" + b"\n" + b"hello\n" + b"------=_x--\n" +) + + +def test_clean_message_raises_no_ambiguity_defect(): + """The markers must be quiet on well-formed mail, or they're noise.""" + assert defects_of(CLEAN) == [] + + +@pytest.mark.parametrize( + "encoding", + [ + pytest.param(b"7bit", id="7bit"), + pytest.param(b"8bit", id="8bit"), + pytest.param(b"binary", id="binary"), + pytest.param(b"base64", id="base64"), + pytest.param(b"quoted-printable", id="quoted-printable"), + pytest.param(b"BASE64", id="uppercase"), + pytest.param(b" base64 ", id="padded"), + ], +) +def test_known_transfer_encodings_are_not_flagged(encoding): + raw = ( + b"From: a@b.co\nSubject: t\nContent-Type: text/plain\n" + b"Content-Transfer-Encoding: " + encoding + b"\n\nx\n" + ) + assert "UnrecognizedTransferEncodingDefect" not in defects_of(raw) + + +def test_duplicate_content_type(): + """We take the first; clients have been seen honouring the second, + which turns a flat text body into a multipart tree whose attachments + we never extract.""" + raw = ( + b"From: a@b.co\nSubject: t\nMIME-Version: 1.0\n" + b"Content-Type: text/plain\n" + b'Content-Type: multipart/mixed; boundary="----=_x"\n' + b"\n------=_x\n\nsecond\n------=_x--\n" + ) + assert "DuplicateContentTypeDefect" in defects_of(raw) + + +def test_duplicate_transfer_encoding(): + """The paper's D3: base64 first, 7bit second smuggled past a filter + that took the second while the clients took the first.""" + raw = ( + b"From: a@b.co\nSubject: t\nContent-Type: text/plain\n" + b"Content-Transfer-Encoding: base64\n" + b"Content-Transfer-Encoding: 7bit\n\nU01VR0dMRUQ=\n" + ) + assert "DuplicateTransferEncodingDefect" in defects_of(raw) + + +@pytest.mark.parametrize( + "raw", + [ + pytest.param( + b"From: a@b.co\nSubject: t\nContent-Type: text/plain\n" + b"Content-Transfer-Encoding: bas64\n\nU01VR0dMRUQ=\n", + id="near-miss-token", + ), + pytest.param( + b"From: a@b.co\nSubject: t\nContent-Type: text/plain\n" + b"Content-Transfer-Encoding:: base64\n\nU01VR0dMRUQ=\n", + id="extra-colon", + ), + ], +) +def test_unrecognized_transfer_encoding(raw): + """We leave the body undecoded. Lenient clients guess — ClamAV decodes + anything base64-ish, Evolution parses past the extra colon — so the + payload is visible to them and not to a scanner reading our output.""" + assert "UnrecognizedTransferEncodingDefect" in defects_of(raw) + + +def test_non_empty_preamble(): + """RFC 2046 §5.1.1 says ignore the preamble, and we do. Thunderbird + and Evolution render it as the message's first line.""" + raw = ( + b"From: a@b.co\nSubject: t\nMIME-Version: 1.0\n" + b'Content-Type: multipart/mixed; boundary="----=_x"\n' + b"\nSMUGGLED\n------=_x\nContent-Type: text/plain\n\nvisible\n------=_x--\n" + ) + assert "NonEmptyPreambleDefect" in defects_of(raw) + # And the smuggled text really is absent from the parsed output. + parsed = parse_email(raw) + assert parsed is not None + assert "SMUGGLED" not in str(parsed["textBody"]) + + +def test_whitespace_only_preamble_is_not_flagged(): + raw = ( + b"From: a@b.co\nSubject: t\nMIME-Version: 1.0\n" + b'Content-Type: multipart/mixed; boundary="----=_x"\n' + b"\n \n------=_x\nContent-Type: text/plain\n\nvisible\n------=_x--\n" + ) + assert "NonEmptyPreambleDefect" not in defects_of(raw) + + +def test_defects_absent_without_extensions(): + """The markers ride on the opt-in extension namespace only.""" + parsed = parse_email(CLEAN) + assert parsed is not None + assert "_ext" not in parsed + + +class TestHeaderAmbiguity: + """Root-level duplications. RFC 5322 §3.6 caps these at one each.""" + + def test_duplicate_from_is_called_out_separately(self): + """A second ``From`` shows one identity to the filter that + authenticated the message and another to the human reading it — + CERT VU#517845, and "Weak Links in Authentication Chains" + (USENIX 2020), which is already in the defense matrix.""" + raw = ( + b"From: alice@good.co\nFrom: mallory@evil.co\n" + b"Subject: t\nMIME-Version: 1.0\nContent-Type: text/plain\n\nx\n" + ) + assert "DuplicateFromDefect" in defects_of(raw) + + @pytest.mark.parametrize( + "header", + [b"Subject", b"Date", b"Message-ID", b"To", b"Reply-To", b"References"], + ) + def test_duplicate_singleton_headers(self, header): + raw = ( + b"From: a@b.co\nMIME-Version: 1.0\nContent-Type: text/plain\n" + + header + + b": one\n" + + header + + b": two\n\nx\n" + ) + assert "DuplicateScalarHeaderDefect" in defects_of(raw) + + def test_nested_message_from_is_not_flagged(self): + """An attached ``message/rfc822`` legitimately carries its own + ``From``; flagging every forwarded email would be noise.""" + raw = ( + b"From: a@b.co\nSubject: t\nMIME-Version: 1.0\n" + b'Content-Type: multipart/mixed; boundary="x"\n\n--x\n' + b"Content-Type: message/rfc822\n\n" + b"From: inner@c.co\nSubject: inner\n\ninner body\n--x--\n" + ) + assert "DuplicateFromDefect" not in defects_of(raw) + + +class TestStructuralAmbiguity: + def test_non_empty_epilogue(self): + """The preamble's mirror image: RFC 2046 §5.1 says ignore both.""" + raw = ( + b"From: a@b.co\nSubject: t\nMIME-Version: 1.0\n" + b'Content-Type: multipart/mixed; boundary="x"\n\n--x\n' + b"Content-Type: text/plain\n\nvisible\n--x--\nSMUGGLED\n" + ) + assert "NonEmptyEpilogueDefect" in defects_of(raw) + + def test_duplicate_boundary_parameter(self): + """Whichever boundary we honour, the other delimits parts we never + see (Inbox Invasion, CCS '24).""" + raw = ( + b"From: a@b.co\nSubject: t\nMIME-Version: 1.0\n" + b'Content-Type: multipart/mixed; boundary="x"; boundary="y"\n\n--x\n' + b"Content-Type: text/plain\n\nfrom-x\n--x--\n" + ) + assert "DuplicateBoundaryParameterDefect" in defects_of(raw) + + def test_missing_mime_version(self): + """MIME syntax with nothing licensing it: a strict receiver reads + the body as flat text and never sees the parts.""" + raw = ( + b"From: a@b.co\nSubject: t\n" + b'Content-Type: multipart/mixed; boundary="x"\n\n--x\n' + b"Content-Type: text/plain\n\nhello\n--x--\n" + ) + assert "MissingMimeVersionDefect" in defects_of(raw) + + +class TestAttachmentNameAmbiguity: + """A part that names itself twice, differently.""" + + def _one_part(self, headers: bytes) -> bytes: + return ( + b"From: a@b.co\nSubject: t\nMIME-Version: 1.0\n" + b'Content-Type: multipart/mixed; boundary="x"\n\n--x\n' + + headers + + b"\nAAA\n--x--\n" + ) + + def test_content_type_name_disagrees_with_filename(self): + raw = self._one_part( + b'Content-Type: application/octet-stream; name="evil.exe"\n' + b'Content-Disposition: attachment; filename="safe.txt"\n' + ) + assert "ConflictingAttachmentNameDefect" in defects_of(raw) + + def test_filename_star_wins_per_rfc6266(self): + """RFC 6266 §4.3: with both present, recipients SHOULD take + ``filename*``. The stdlib's ``get_filename`` returns the plain + one, so a sender could show us ``safe.txt`` while every + spec-following client saved ``evil.exe``.""" + raw = self._one_part( + b"Content-Type: application/octet-stream\n" + b"Content-Disposition: attachment; " + b"filename=\"safe.txt\"; filename*=UTF-8''evil.exe\n" + ) + parsed = parse_email(raw) + assert parsed is not None + assert [a["name"] for a in parsed["attachments"]] == ["evil.exe"] + assert "ConflictingAttachmentNameDefect" in defects_of(raw) + + def test_agreeing_names_are_not_flagged(self): + raw = self._one_part( + b'Content-Type: application/pdf; name="r.pdf"\n' + b'Content-Disposition: attachment; filename="r.pdf"\n' + ) + assert "ConflictingAttachmentNameDefect" not in defects_of(raw) + + +class TestDecodeTimeDefectsAreCollected: + """Regression: the stdlib attaches these while *decoding* a payload, + and the collection walk used to run before any decoding happened, so + every one of them was silently dropped from ``_ext.defects``.""" + + @pytest.mark.parametrize( + ("payload", "expected"), + [ + pytest.param( + b"U01VR0!!dMRUQ=", "InvalidBase64CharactersDefect", id="chars" + ), + pytest.param(b"U01VR0dMRUQ", "InvalidBase64PaddingDefect", id="padding"), + ], + ) + def test_base64_decode_defects_reach_ext(self, payload, expected): + raw = ( + b"From: a@b.co\nSubject: t\nMIME-Version: 1.0\n" + b"Content-Type: text/plain\nContent-Transfer-Encoding: base64\n\n" + + payload + + b"\n" + ) + assert expected in defects_of(raw) + + +class TestIetfDraftAnomalyClasses: + """The classes named by draft-chen-email-mime-ambiguity-defense. + + That draft enumerates the constructs an ingress filter should treat + as ambiguous rather than resolve silently. These are the ones the + stdlib does not already flag for us. + """ + + def test_control_char_in_mime_header(self): + """A NUL in a filename is read three ways: stripped (us), + truncated at the NUL, or kept. Same part, three names.""" + raw = ( + b"From: a@b.co\nSubject: t\nMIME-Version: 1.0\n" + b'Content-Type: multipart/mixed; boundary="x"\n\n--x\n' + b"Content-Type: application/octet-stream\n" + b'Content-Disposition: attachment; filename="report.pdf\x00.exe"\n' + b"\nAAA\n--x--\n" + ) + assert "ControlCharInHeaderDefect" in defects_of(raw) + + @pytest.mark.parametrize( + "content_type", + [ + pytest.param(b'multipart/mixed; boundary=""', id="empty"), + pytest.param(b"multipart/mixed", id="absent"), + ], + ) + def test_empty_or_missing_boundary(self, content_type): + raw = ( + b"From: a@b.co\nSubject: t\nMIME-Version: 1.0\nContent-Type: " + + content_type + + b"\n\n--x\nContent-Type: text/plain\n\nhi\n--x--\n" + ) + assert "EmptyBoundaryDefect" in defects_of(raw) + + def test_encoded_word_in_parameter(self): + """RFC 2231 is the mechanism for non-ASCII parameters; RFC 2047 + encoded-words are not permitted there. We decode them, so a + scanner that doesn't sees a different filename than the user.""" + raw = ( + b"From: a@b.co\nSubject: t\nMIME-Version: 1.0\n" + b'Content-Type: multipart/mixed; boundary="x"\n\n--x\n' + b"Content-Type: application/pdf\n" + b'Content-Disposition: attachment; filename="=?utf-8?B?ZXZpbC5leGU=?="\n' + b"\nAAA\n--x--\n" + ) + assert "EncodedWordInParameterDefect" in defects_of(raw) + parsed = parse_email(raw) + assert parsed is not None + assert [a["name"] for a in parsed["attachments"]] == ["evil.exe"] + + def test_rfc2231_parameter_is_not_flagged(self): + """The correct mechanism must not trip the marker.""" + raw = ( + b"From: a@b.co\nSubject: t\nMIME-Version: 1.0\n" + b'Content-Type: multipart/mixed; boundary="x"\n\n--x\n' + b"Content-Type: application/pdf\n" + b"Content-Disposition: attachment; filename*=UTF-8''r%C3%A9sum%C3%A9.pdf\n" + b"\nAAA\n--x--\n" + ) + assert "EncodedWordInParameterDefect" not in defects_of(raw) + + +class TestOpaqueAndAmbiguousPartMarkers: + """Markers for parts we hand over without looking inside, and for the + one fold that changes an attachment's name. + + Motivated by *Inbox Invasion* (CCS '24) and the nested-RFC822 / + CRLF-filename technique reported in the wild: the payload is placed + where the reader's parser finds it and the scanner's does not. + """ + + @staticmethod + def _wrap(part_headers: bytes, body: bytes = b"MZ") -> bytes: + return ( + b"From: o@x.co\r\nTo: a@b.co\r\nSubject: s\r\nMIME-Version: 1.0\r\n" + b"Content-Type: multipart/mixed; boundary=B\r\n\r\n--B\r\n" + + part_headers + + b"\r\n\r\n" + + body + + b"\r\n--B--\r\n" + ) + + @staticmethod + def _defects(raw: bytes) -> set[str]: + parsed = parse_email(raw, extensions=True) + assert parsed is not None + return set((parsed.get("_ext") or {}).get("defects") or []) + + def test_fold_inside_a_quoted_filename_is_flagged(self): + """``filename="pay load.exe"`` is read three ways: we unfold + to ``pay load.exe`` per RFC 5322 §2.2.3, a parser dropping the + whole fold sees ``payload.exe``, one truncating at CR sees + ``pay``. Legal syntax, three names, one attachment.""" + raw = self._wrap( + b"Content-Type: application/octet-stream\r\n" + b'Content-Disposition: attachment; filename="pay\r\n load.exe"' + ) + assert "FoldInQuotedParameterDefect" in self._defects(raw) + parsed = parse_email(raw) + assert parsed["attachments"][0]["name"] == "pay load.exe" + + def test_ordinary_folding_between_parameters_is_not_flagged(self): + """Folding is how long headers are written; only folding *inside* + the quotes is ambiguous. Flagging both would be noise.""" + raw = self._wrap( + b"Content-Type: application/octet-stream\r\n" + b"Content-Disposition: attachment;\r\n" + b' filename="report.pdf"' + ) + assert "FoldInQuotedParameterDefect" not in self._defects(raw) + + def test_message_partial_is_flagged(self): + """RFC 2046 §5.2.2 splits one message across several, so the + payload exists only after a reassembly a per-message scanner + never performs.""" + raw = ( + b"From: o@x.co\r\nTo: a@b.co\r\nSubject: s\r\nMIME-Version: 1.0\r\n" + b'Content-Type: message/partial; id="x@y"; number=1; total=2\r\n\r\n' + b"From: inner@x.co\r\nSubject: half\r\n\r\nfragment\r\n" + ) + assert "PartialMessageDefect" in self._defects(raw) + + def test_message_external_body_is_flagged(self): + """The content is fetched from elsewhere, so it is not in this + message for anyone to scan.""" + raw = self._wrap( + b"Content-Type: message/external-body; access-type=URL;" + b' URL="http://evil.co/p.exe"', + body=b"Content-Type: application/octet-stream\r\n\r\n", + ) + assert "ExternalBodyDefect" in self._defects(raw) + + def test_nested_rfc822_is_deliberately_not_flagged(self): + """The documented scope boundary: a marker here would fire on + every forwarded message, so it would be noise rather than signal. + Pinned so the decision is visible rather than accidental — the + nested payload is still reachable by re-parsing ``content``.""" + inner = b"From: i@x.co\r\nSubject: inner\r\n\r\nbody\r\n" + raw = self._wrap( + b"Content-Type: message/rfc822\r\nContent-Disposition: attachment", + body=inner, + ) + defects = self._defects(raw) + assert "PartialMessageDefect" not in defects + assert "ExternalBodyDefect" not in defects + # And the bytes are still there to recurse into. + parsed = parse_email(raw) + nested = parsed["attachments"][0] + assert nested["type"] == "message/rfc822" + assert parse_email(nested["content"])["subject"] == "inner" + + def test_ordinary_attachment_raises_no_marker(self): + """The markers are only worth anything if normal mail is clean.""" + raw = self._wrap( + b"Content-Type: application/pdf\r\n" + b'Content-Disposition: attachment; filename="report.pdf"' + ) + assert self._defects(raw) == set() + + +class TestStdlibEmailCveRegressions: + """Behaviour we inherit from CPython, pinned because the 3.14.6 floor + exists precisely to carry ``email`` fixes — a downgrade or a vendored + stdlib would reintroduce these silently.""" + + def test_cve_2025_1795_address_list_folding_keeps_its_commas(self): + """A comma separator landing on a folded, unicode-encoded line was + itself RFC 2047-encoded, so receivers merged or split recipients. + The invariant is arithmetic: what goes in comes out.""" + recipients = [ + { + "name": f"Zoé Ünicode Nom Très Long Numéro {i}", + "email": f"r{i}@example.com", + } + for i in range(8) + ] + raw = compose_email( + { + "from": [{"name": "Émetteur", "email": "s@e.co"}], + "to": recipients, + "subject": "sujet", + "sentAt": "2026-01-01T00:00:00+00:00", + "textBody": [{"content": "b"}], + } + ) + # The header must actually fold, or the test proves nothing. + to_header = raw.decode().split("\r\nTo: ")[1].split("\r\nSubject")[0] + assert "\r\n" in to_header + recovered = parse_email(raw).get("to") or [] + assert [a["email"] for a in recovered] == [r["email"] for r in recipients] + + @pytest.mark.parametrize( + "payload", + [ + pytest.param("evil\r\nBcc: x@y.co", id="crlf"), + pytest.param("evil\nBcc: x@y.co", id="lf"), + pytest.param("evil\rBcc: x@y.co", id="cr"), + ], + ) + def test_cve_2024_6923_newline_in_header_cannot_inject(self, payload): + """The stdlib failed to quote newlines in header values. We strip + them before they reach the header machinery, so neither layer + alone is load-bearing.""" + raw = compose_email( + { + "from": [{"name": payload, "email": "s@e.co"}], + "to": [{"name": None, "email": "a@b.co"}], + "subject": payload, + "sentAt": "2026-01-01T00:00:00+00:00", + "textBody": [{"content": "b"}], + } + ) + names = {h["name"].lower() for h in (parse_email(raw).get("headers") or [])} + assert "bcc" not in names + + +class TestDefectCollectionSurvivesBrokenAccessors: + """A damaged header block must not cost the *other* markers. + + ``get_params()`` can raise on sufficiently broken input. Returning at + that point would mean the most damaged messages — the ones a + quarantine policy most wants flagged — come back with the fewest + markers, which is exactly backwards. + """ + + @staticmethod + def _raising_part(monkeypatch, real): + def boom(*_a, **_k): + raise ValueError("damaged header block") + + monkeypatch.setattr(type(real), "get_params", boom, raising=False) + return real + + def test_later_markers_still_fire_when_get_params_raises(self, monkeypatch): + import email as _email + from email import policy as _policy + + from jmap_email.parser import _collect_ambiguity_defects + + raw = ( + b'Content-Type: message/partial; id="x@y"; number=1; total=2\r\n' + b"Content-Transfer-Encoding: bas64\r\n\r\nbody\r\n" + ) + part = _email.message_from_bytes(raw, policy=_policy.compat32) + self._raising_part(monkeypatch, part) + + defects: list[str] = [] + _collect_ambiguity_defects(part, defects) + + # Both of these are read from sources other than get_params(), so + # they must survive its failure. + assert "UnrecognizedTransferEncodingDefect" in defects + assert "PartialMessageDefect" in defects + + def test_unreadable_boundary_counts_as_absent(self, monkeypatch): + """Unreadable is not better than missing: either way nobody can + agree where the parts start.""" + import email as _email + from email import policy as _policy + + from jmap_email.parser import _collect_ambiguity_defects + + raw = b"Content-Type: multipart/mixed; boundary=B\r\n\r\nbody\r\n" + part = _email.message_from_bytes(raw, policy=_policy.compat32) + + def boom(*_a, **_k): + raise ValueError("damaged parameter") + + monkeypatch.setattr(type(part), "get_param", boom, raising=False) + + defects: list[str] = [] + _collect_ambiguity_defects(part, defects) + assert "EmptyBoundaryDefect" in defects diff --git a/src/jmap-email/tests/test_composer.py b/src/jmap-email/tests/test_composer.py index e6ca577c..500dca0f 100644 --- a/src/jmap-email/tests/test_composer.py +++ b/src/jmap-email/tests/test_composer.py @@ -16,7 +16,15 @@ from email.parser import BytesParser import pytest import jmap_email.composer as _composer_module -from jmap_email import is_valid_msg_id, parse_email +from jmap_email import ( + DEFAULT_COMPOSE_OPTIONS, + ComposeOptions, + is_valid_addr_spec, + is_valid_msg_id, + parse_address, + parse_email, + preview_text, +) from jmap_email.composer import ( _MSG_ID_MAX_OCTETS, _POLICY, @@ -328,10 +336,10 @@ class TestEmailComposition: "textBody": [{"content": "Hello everyone!"}], } - # keep_bcc=True so this contract test exercises the full address-list + # emit_bcc=True so this contract test exercises the full address-list # serialization path including Bcc. Default behavior (Bcc dropped) is # covered by TestComposerRFCAudit.test_bcc_dropped_by_default. - result_bytes = compose_email(jmap_data, keep_bcc=True) + result_bytes = compose_email(jmap_data, options=ComposeOptions(emit_bcc=True)) assert isinstance(result_bytes, bytes) parsed = BytesParser().parsebytes(result_bytes) @@ -673,7 +681,7 @@ class TestEmailComposition: ], } - raw_email = compose_email(jmap_data, keep_bcc=True) + raw_email = compose_email(jmap_data, options=ComposeOptions(emit_bcc=True)) msg = email.message_from_bytes(raw_email, policy=policy.default) part = next(p for p in msg.walk() if p.get_filename() == "details.txt") @@ -709,7 +717,7 @@ class TestEmailComposition: ], } - raw_email = compose_email(jmap_data, keep_bcc=True) + raw_email = compose_email(jmap_data, options=ComposeOptions(emit_bcc=True)) msg = email.message_from_bytes(raw_email, policy=policy.default) part = next(p for p in msg.walk() if p.get_filename() == "details.txt") @@ -1222,8 +1230,13 @@ class TestErrorHandling: # Test with empty strings assert format_address("", "") == "" - # Test with unusual email format (missing domain) - assert "user-without-domain" in format_address("Test", "user-without-domain") + # A value that is not an addr-spec yields "" rather than being + # emitted: format_address's own docstring points callers at + # envelope construction and quoted-block headers, so anything it + # returns may end up in a header and must be a real mailbox. + assert format_address("Test", "user-without-domain") == "" + # Nor may it mint a second mailbox out of one entry. + assert format_address("Test", "a@b.co, evil@x.co") == "" # Test with extremely long name long_name = "A" * 100 @@ -1250,7 +1263,9 @@ class TestErrorHandling: "cid": case["cid"], } - attachment_part = _create_attachment_part(attachment) + attachment_part = _create_attachment_part( + attachment, DEFAULT_COMPOSE_OPTIONS + ) # Verify the attachment part was created assert attachment_part is not None @@ -1708,27 +1723,28 @@ class TestComposerRFCAudit: # pylint: disable=too-many-public-methods # --- B. CR/LF in the email address portion (CVE-2021-23400 nodemailer) - def test_crlf_in_email_address_does_not_inject_header(self): - """A \\r\\n smuggled into the email field must not survive into the wire bytes. + """A \\r\\n smuggled into the email field is now refused outright. - format_address only .strip()s whitespace from the email field. The - guarantee comes from _sanitize_header_value wrapping the formatted - result. Lock that contract here. + Stripping the CRLF leaves ``a@b.comBcc: evil@evil.tld`` — no + injection, but not a mailbox either, and emitting it would put + nonsense in a To: header. Strict compose rejects instead, which + is the stronger property: the bytes are never built at all. """ - raw, parsed = self._compose_and_parse( - self._minimal(to=[{"name": "x", "email": "a@b.com\r\nBcc: evil@evil.tld"}]) - ) - assert b"evil@evil.tld" not in raw or b"Bcc:" not in raw - # Bcc must not appear as a separate header - assert parsed["Bcc"] is None + with pytest.raises(InvalidAddressError): + compose_email( + self._minimal( + to=[{"name": "x", "email": "a@b.com\r\nBcc: evil@evil.tld"}] + ) + ) def test_crlf_in_from_email_does_not_inject_header(self): """Same CRLF-in-email guard applied via the From path.""" - raw, _ = self._compose_and_parse( - self._minimal(**{"from": [{"name": "n", "email": "a@b\r\nX-Injected: 1"}]}) - ) - # injected line must be folded into something parsing can't split on - msg = email.message_from_bytes(raw) - assert msg["X-Injected"] is None + with pytest.raises(InvalidAddressError): + compose_email( + self._minimal( + **{"from": [{"name": "n", "email": "a@b\r\nX-Injected: 1"}]} + ) + ) # --- C. Display-name with control chars (RFC 5322 atext) --------------- @@ -1981,19 +1997,19 @@ class TestComposerRFCAudit: # pylint: disable=too-many-public-methods def test_bcc_dropped_by_default(self): """RFC 5322 §3.6.3: Bcc must not be transmitted to recipients. The composer drops it by default. Only archive-reconstruction callers - (PST import) opt in via keep_bcc=True.""" + (PST import) opt in via emit_bcc=True.""" raw, parsed = self._compose_and_parse( self._minimal(bcc=[{"name": "BCC R", "email": "bcc@example.com"}]) ) assert parsed["Bcc"] is None assert b"bcc@example.com" not in raw - def test_bcc_emitted_when_keep_bcc_true(self): - """Archive-reconstruction opt-in: PST import passes keep_bcc=True so + def test_bcc_emitted_when_emit_bcc_true(self): + """Archive-reconstruction opt-in: PST import passes emit_bcc=True so the original Bcc list is preserved in the stored .eml.""" raw = compose_email( self._minimal(bcc=[{"name": "BCC R", "email": "bcc@example.com"}]), - keep_bcc=True, + options=ComposeOptions(emit_bcc=True), ) parsed = BytesParser(policy=policy.default).parsebytes(raw) assert parsed["Bcc"] is not None @@ -3116,3 +3132,903 @@ class TestIsValidMsgId: if __name__ == "__main__": pytest.main() + + +class TestAddressInjectionViaCompose: + """Address-shaped values that must not become mailboxes. + + All three were found by the parse/compose round-trip fuzz suite: the + composed bytes were re-parsed and compared against what was supplied. + """ + + def _minimal_jmap(self, **over): + base = { + "from": [{"name": None, "email": "a@b.co"}], + "to": [{"name": None, "email": "c@d.co"}], + "subject": "s", + "sentAt": "2026-06-08T12:00:00+00:00", + "textBody": [{"partId": "1", "type": "text/plain", "content": "x"}], + } + base.update(over) + return base + + def test_encoded_word_display_name_stays_one_mailbox(self): + """A display name that decodes to an address must be quoted. + + ``=?utf-8?B?ZXZpbEB4LmNv?=`` carries none of the characters the + quoting check looks for, but the header machinery decodes it + afterwards to ``evil@x.co`` — emitted bare that is a second + mailbox, so an attacker-chosen display name became a recipient. + """ + raw = compose_email( + self._minimal_jmap( + to=[{"name": "=?utf-8?B?ZXZpbEB4LmNv?=", "email": "a@b.co"}] + ) + ) + reparsed = parse_email(raw) + assert [a["email"] for a in reparsed["to"]] == ["a@b.co"] + + def test_comma_in_addr_spec_is_refused(self): + """One ``email`` value, one mailbox — never two.""" + with pytest.raises(InvalidAddressError): + compose_email( + self._minimal_jmap(to=[{"name": "x", "email": "a@b.co, evil@x.co"}]) + ) + + def test_non_ascii_addr_spec_is_refused(self): + """RFC 6531 addresses need SMTPUTF8, which this composer doesn't + emit. Left alone the stdlib RFC 2047-encodes the addr-spec — + forbidden by RFC 2047 §5, unroutable, and on re-parse the + display name wins as the recipient. + """ + with pytest.raises(InvalidAddressError): + compose_email(self._minimal_jmap(to=[{"name": "x", "email": "é@ü.co"}])) + + def test_boundaries_are_unpredictable(self): + """Boundaries come from a CSPRNG, not the stdlib's Mersenne + Twister: predicting one is what lets a sender who controls part + of a body close our part and append their own.""" + jmap = self._minimal_jmap( + htmlBody=[{"partId": "2", "type": "text/html", "content": "

x

"}] + ) + boundaries = { + email.message_from_bytes(compose_email(jmap)).get_boundary() + for _ in range(8) + } + assert len(boundaries) == 8 + assert all(b and " " not in b for b in boundaries) + + +class TestHistoricalCVERegressions: + """Address-parsing CVE classes from the past decade of CPython. + + Each was reachable through this library before the addr-spec + predicate existed; the defense matrix now names them. + """ + + @pytest.mark.parametrize( + "addr", + [ + pytest.param("a@b@evil.co", id="two-at"), + pytest.param("victim@good.co@evil.co", id="domain-looks-allowed"), + pytest.param("a@@b.co", id="empty-middle"), + pytest.param("a@b.co@", id="trailing-at"), + ], + ) + def test_cve_2019_16056_multiple_at_is_rejected(self, addr): + """CPython's parseaddr mis-split addresses with several ``@``, so + an allowlist keyed on the domain could be talked into accepting + one it meant to deny. An unquoted ``@`` in the local-part is not + an addr-spec, and is refused.""" + assert is_valid_addr_spec(addr) is False + assert parse_address(addr) == ("", "") + + def test_quoted_local_part_keeps_its_at(self): + """``"a@b.co"@evil.co`` *is* one mailbox: the quoting makes the + inner ``@`` data, and the domain is unambiguous.""" + assert is_valid_addr_spec('"a@b.co"@evil.co') is True + + def test_cve_2023_36632_nested_comments_do_not_recurse(self): + """A crafted argument drove parseaddr into RecursionError; we + catch and degrade rather than let it escape.""" + assert parse_address("(" * 50000 + "a@b.co") == ("", "") + + @pytest.mark.parametrize( + "body", + [ + pytest.param("before\r\n.\r\nafter", id="crlf-dot"), + pytest.param("before\n.\nafter", id="bare-lf-dot"), + pytest.param("a\nb\nc", id="bare-lf"), + pytest.param("a\rb", id="bare-cr"), + ], + ) + def test_cve_2023_51764_output_is_crlf_canonical(self, body): + """SMTP smuggling works because some servers accept ``\\n.\\n`` as + a DATA terminator. A composer that emitted a bare LF would be + *generating* that vector out of body text an attacker chose, so + every line ending in our output must be a full CRLF. + """ + raw = compose_email( + { + "from": [{"name": None, "email": "a@b.co"}], + "to": [{"name": None, "email": "c@d.co"}], + "subject": "s", + "sentAt": "2026-06-08T12:00:00+00:00", + "textBody": [{"partId": "1", "type": "text/plain", "content": body}], + } + ) + assert b"\n" not in raw.replace(b"\r\n", b"") + assert b"\r" not in raw.replace(b"\r\n", b"") + + +class TestQuotedLocalPartTermination: + """A quoted-string local-part must actually close. + + Stripping ``\\\\`` and ``\\"`` pairs and checking for a leftover quote + is not equivalent to walking escapes: it accepts a local-part ending + in a lone backslash — in ``"a\\"`` the backslash escapes the closing + DQUOTE — so the string never terminates and keeps quoting whatever + the header carries after it. + """ + + @pytest.mark.parametrize( + "addr", + [ + # The backslash escapes the closing quote, so the string runs on. + pytest.param('"a\\"@b.co', id="trailing-escape"), + # Nothing but the escape: same shape, empty content. + pytest.param('"\\"@b.co', id="escape-only"), + # Escaped backslash, then an escaped quote — still unterminated. + pytest.param('"a\\\\\\"@b.co', id="escaped-pair-then-escape"), + # An unescaped interior quote closes early; the rest is loose. + pytest.param('"a"b"@b.co', id="early-close"), + ], + ) + def test_unterminated_quoted_string_is_rejected(self, addr): + assert is_valid_addr_spec(addr) is False + + @pytest.mark.parametrize( + "addr", + [ + pytest.param('"john doe"@e.co', id="space-inside"), + pytest.param('"a@b.co"@e.co', id="at-inside"), + pytest.param('"a,b"@e.co', id="comma-inside"), + # An escaped backslash is a complete quoted-pair: this closes. + pytest.param('"a\\\\"@e.co', id="escaped-backslash"), + # An escaped quote is content, not a terminator. + pytest.param('"a\\"b"@e.co', id="escaped-quote"), + # RFC 5322 permits zero qcontent. + pytest.param('""@e.co', id="empty-quoted-string"), + ], + ) + def test_terminated_quoted_string_is_accepted(self, addr): + assert is_valid_addr_spec(addr) is True + + def test_unterminated_local_part_cannot_reach_the_wire(self): + """The composer refuses it rather than emitting a header whose + mailbox count depends on who is reading.""" + with pytest.raises(InvalidAddressError): + compose_email( + { + "from": [{"name": None, "email": "s@e.co"}], + "to": [ + {"name": None, "email": '"a\\"@b.co'}, + {"name": None, "email": "victim@x.co"}, + ], + "subject": "s", + "sentAt": "2026-01-01T00:00:00+00:00", + "textBody": [{"content": "b"}], + } + ) + + def test_accepted_quoted_local_part_stays_one_mailbox(self): + """The flip side: what the predicate *does* accept must still + count as exactly one mailbox once it sits next to another + recipient in a real mailbox-list.""" + raw = compose_email( + { + "from": [{"name": None, "email": "s@e.co"}], + "to": [ + {"name": None, "email": '"a,b"@e.co'}, + {"name": None, "email": "victim@x.co"}, + ], + "subject": "s", + "sentAt": "2026-01-01T00:00:00+00:00", + "textBody": [{"content": "b"}], + } + ) + parsed = BytesParser(policy=_POLICY).parsebytes(raw) + pairs = email.utils.getaddresses([str(parsed["To"])]) + assert [addr for _name, addr in pairs] == ['"a,b"@e.co', "victim@x.co"] + + +class TestComposeOptionsIdnaDomains: + """``idna_encode_domains`` governs the domain, and only the domain. + + The split is the whole point of the option. Punycode is a DNS + algorithm (RFC 3492/5891) and the local part is not a DNS label, so + an IDN domain has an ASCII wire form and a non-ASCII local part does + not. Carrying the latter needs SMTPUTF8 (RFC 6531), negotiated + per-hop against the receiver's EHLO with no downgrade path — RFC 6530 + dropped the RFC 5504 mechanism — and this composer emits 7-bit and + never negotiates it. A flag covering both would silently produce + undeliverable mail. + + Note the stdlib does none of this: ``email.policy.default`` RFC + 2047-encodes a non-ASCII domain, producing + ``contact@=?utf-8?q?exempl=C3=A9?=.fr`` — which RFC 2047 §5 forbids + inside an addr-spec and no MTA routes. + """ + + @staticmethod + def _jmap(email_addr): + return { + "from": [{"name": "S", "email": "s@e.co"}], + "to": [{"name": "N", "email": email_addr}], + "subject": "s", + "sentAt": "2026-01-01T00:00:00+00:00", + "textBody": [{"content": "b"}], + } + + def test_idn_domain_is_refused_by_default(self): + """Strict default: the composer does not rewrite an address it + was handed unless asked to.""" + with pytest.raises(InvalidAddressError) as excinfo: + compose_email(self._jmap("contact@exemplé.fr")) + # The error has to name the way out, or an outside consumer has + # no path from the exception to the option. + assert "idna_encode_domains=True" in str(excinfo.value) + + def test_idn_domain_is_encoded_when_enabled(self): + raw = compose_email( + self._jmap("contact@exemplé.fr"), + options=ComposeOptions(idna_encode_domains=True), + ) + parsed = BytesParser(policy=_POLICY).parsebytes(raw) + assert "contact@xn--exempl-gva.fr" in str(parsed["To"]) + # Pure ASCII on the wire: that is the point of the conversion. + assert raw.isascii() + + @pytest.mark.parametrize( + "addr", + [ + pytest.param("josé@exemple.fr", id="non-ascii-local"), + pytest.param("josé@exemplé.fr", id="non-ascii-both"), + ], + ) + def test_non_ascii_local_part_raises_whatever_the_flag(self, addr): + for options in ( + ComposeOptions(idna_encode_domains=False), + ComposeOptions(idna_encode_domains=True), + ): + with pytest.raises(InvalidAddressError) as excinfo: + compose_email(self._jmap(addr), options=options) + assert "SMTPUTF8" in str(excinfo.value) + + def test_domain_with_no_idna_encoding_is_refused(self): + """A label over 63 octets has no legal A-label form. Refusing + beats emitting a domain that cannot resolve.""" + with pytest.raises(InvalidAddressError): + compose_email( + self._jmap("a@" + "é" * 70 + ".fr"), + options=ComposeOptions(idna_encode_domains=True), + ) + + def test_deviation_codepoints_are_not_folded(self): + """UTS 46 non-transitional: ``faß.de`` and ``fass.de`` are + distinct registrable domains (DENIC allows ß since 2010), so + the encoder must not fold one into the other the way IDNA2003 + nameprep silently did — that misdirected the mail.""" + raw = compose_email( + self._jmap("a@faß.de"), + options=ComposeOptions(idna_encode_domains=True), + ) + assert b"a@xn--fa-hia.de" in raw + assert b"fass.de" not in raw + + def test_trailing_root_dot_is_refused(self): + """``idna.encode`` keeps a trailing root dot, but a domain + ending in ``.`` is not a valid RFC 5322 dot-atom, so the + composer keeps refusing it.""" + with pytest.raises(InvalidAddressError): + compose_email( + self._jmap("a@exemplé.fr."), + options=ComposeOptions(idna_encode_domains=True), + ) + + def test_ascii_address_is_untouched(self): + """The flag must be inert for the overwhelmingly common case.""" + for flag in (False, True): + raw = compose_email( + self._jmap("plain@example.com"), + options=ComposeOptions(idna_encode_domains=flag), + ) + parsed = BytesParser(policy=_POLICY).parsebytes(raw) + assert "plain@example.com" in str(parsed["To"]) + + def test_caller_input_is_not_mutated(self): + """A JMAP dict handed to compose_email is frequently the same + object the caller goes on to store, so the rewrite must land on + a copy.""" + jmap = self._jmap("contact@exemplé.fr") + compose_email(jmap, options=ComposeOptions(idna_encode_domains=True)) + assert jmap["to"][0]["email"] == "contact@exemplé.fr" + + def test_every_address_field_is_covered(self): + """A recipient hidden in cc/bcc/replyTo must not skip the check.""" + for field in ("sender", "replyTo", "cc", "bcc"): + jmap = self._jmap("ok@e.co") + jmap[field] = [{"name": None, "email": "contact@exemplé.fr"}] + with pytest.raises(InvalidAddressError) as excinfo: + compose_email(jmap) + assert field in str(excinfo.value) + + +class TestComposeOptionsDefaults: + """``DEFAULT_COMPOSE_OPTIONS`` is what ``options=None`` resolves to.""" + + def test_default_instance_matches_an_empty_construction(self): + assert DEFAULT_COMPOSE_OPTIONS == ComposeOptions() + + def test_defaults_preserve_the_pre_bundle_behaviour(self): + """The shipped defaults: nothing assumed about the hop, no + rewriting of caller data, no Bcc on the wire.""" + assert DEFAULT_COMPOSE_OPTIONS.emit_bcc is False + assert DEFAULT_COMPOSE_OPTIONS.idna_encode_domains is False + assert DEFAULT_COMPOSE_OPTIONS.allow_8bit is False + assert DEFAULT_COMPOSE_OPTIONS.allow_smtputf8 is False + + def test_frozen_and_hashable_like_parse_options(self): + """Same contract as ParseOptions: safe to build once at import + time and share across threads.""" + options = ComposeOptions(idna_encode_domains=True) + assert hash(options) == hash(ComposeOptions(idna_encode_domains=True)) + with pytest.raises(AttributeError): + options.idna_encode_domains = False + + +class TestComposeOptionsEsmtpCapabilities: + """``allow_8bit`` and ``allow_smtputf8`` name capabilities of the *hop*. + + The library never discovers those — the caller does. How is the + caller's business: a relay whose config it owns, an EHLO response it + read before composing, or two variants composed up front so the + delivery loop can fall back. What the library owes is that each flag + produces exactly the wire form the corresponding extension licenses, + and nothing more. + """ + + @staticmethod + def _jmap(**over): + base = { + "from": [{"name": "Alice", "email": "alice@example.com"}], + "to": [{"name": "Bob", "email": "bob@example.com"}], + "subject": "café", + "sentAt": "2026-01-01T00:00:00+00:00", + "textBody": [{"content": "café naïve résumé"}], + } + base.update(over) + return base + + def test_default_is_7bit_and_pure_ascii(self): + """No extension assumed: body encoded down, headers RFC 2047.""" + raw = compose_email(self._jmap()) + assert raw.isascii() + assert b"Content-Transfer-Encoding: base64" in raw + assert b"=?utf-8?q?caf=C3=A9?=" in raw + + def test_allow_8bit_emits_raw_body_but_keeps_2047_headers(self): + """8BITMIME (RFC 6152) licenses 8-bit *body* octets and nothing + else. Headers still need RFC 2047 — that takes SMTPUTF8.""" + raw = compose_email(self._jmap(), options=ComposeOptions(allow_8bit=True)) + assert not raw.isascii() + assert b"Content-Transfer-Encoding: 8bit" in raw + assert "café naïve résumé".encode() in raw + # The subject is still an encoded-word: 8BITMIME says nothing + # about header charsets. + assert b"=?utf-8?q?caf=C3=A9?=" in raw + + def test_smtputf8_emits_utf8_headers(self): + """RFC 6532: headers go out as UTF-8 rather than encoded-words.""" + raw = compose_email(self._jmap(), options=ComposeOptions(allow_smtputf8=True)) + assert "Subject: café".encode() in raw + assert b"=?utf-8?" not in raw + + def test_smtputf8_implies_8bit(self): + """RFC 6531 §3.1 requires an SMTPUTF8 server to support 8BITMIME, + and UTF-8 headers are 8-bit by construction — encoding the body + down while emitting 8-bit headers would buy nothing.""" + assert ComposeOptions(allow_smtputf8=True).emits_8bit is True + raw = compose_email(self._jmap(), options=ComposeOptions(allow_smtputf8=True)) + assert b"Content-Transfer-Encoding: 8bit" in raw + + @pytest.mark.parametrize( + ("allow_8bit", "allow_smtputf8", "expected"), + [ + pytest.param(False, False, False, id="neither"), + pytest.param(True, False, True, id="8bit-only"), + pytest.param(False, True, True, id="utf8-implies-8bit"), + pytest.param(True, True, True, id="both"), + ], + ) + def test_emits_8bit_truth_table(self, allow_8bit, allow_smtputf8, expected): + options = ComposeOptions(allow_8bit=allow_8bit, allow_smtputf8=allow_smtputf8) + assert options.emits_8bit is expected + + def test_smtputf8_permits_a_non_ascii_local_part(self): + """The one thing IDNA cannot do. Under SMTPUTF8 the whole + addr-spec travels as UTF-8, so there is nothing to convert.""" + raw = compose_email( + self._jmap(to=[{"name": "B", "email": "josé@exemplé.fr"}]), + options=ComposeOptions(allow_smtputf8=True), + ) + assert "josé@exemplé.fr".encode() in raw + + def test_non_ascii_local_part_still_refused_without_smtputf8(self): + """Including with idna_encode_domains on — a domain encoding does + not license a local part.""" + for options in ( + ComposeOptions(), + ComposeOptions(allow_8bit=True), + ComposeOptions(idna_encode_domains=True), + ): + with pytest.raises(InvalidAddressError): + compose_email( + self._jmap(to=[{"name": "B", "email": "josé@exemple.fr"}]), + options=options, + ) + + def test_ascii_message_is_byte_identical_across_flags(self): + """The flags must be inert when there is nothing non-ASCII to + encode, so a caller can set them by deployment without changing + the output of ordinary mail.""" + plain = self._jmap(subject="hello", textBody=[{"content": "plain body"}]) + baseline = compose_email(plain) + for options in ( + ComposeOptions(allow_8bit=True), + ComposeOptions(allow_smtputf8=True), + ComposeOptions(allow_8bit=True, allow_smtputf8=True), + ): + other = compose_email(plain, options=options) + # Boundaries are freshly random per compose, and this message + # has no multipart, so the bytes are directly comparable. + assert other == baseline + + def test_two_variants_for_the_delivery_time_fallback(self): + """The pattern the flags exist for: compose both up front, send + the SMTPUTF8 one, fall back on SMTPNotSupportedError. + + This works whenever an ASCII form *exists* — here an IDN domain + with an ASCII local part. When the local part is non-ASCII there + is no ASCII variant to fall back to (RFC 6530 dropped the + downgrade), and the caller must bounce instead; the sibling test + above pins that half. + """ + jmap = self._jmap(to=[{"name": "B", "email": "contact@exemplé.fr"}]) + preferred = compose_email( + jmap, options=ComposeOptions(allow_smtputf8=True, allow_8bit=True) + ) + fallback = compose_email(jmap, options=ComposeOptions(idna_encode_domains=True)) + assert "contact@exemplé.fr".encode() in preferred + assert b"contact@xn--exempl-gva.fr" in fallback + assert fallback.isascii() + + +class TestExtIsNotAComposeInput: + """``_ext`` is parser-only; the composer never reads it. + + Pinned because the parser emits a *typed* ``_ext.resent`` projection + that looks like something you could compose from. You cannot — and + the failure is silent, so it needs a test rather than a comment. + """ + + @staticmethod + def _jmap(**over): + base = { + "from": [{"name": "A", "email": "a@e.co"}], + "to": [{"name": "B", "email": "b@e.co"}], + "subject": "s", + "sentAt": "2026-01-01T00:00:00+00:00", + "textBody": [{"content": "x"}], + } + base.update(over) + return base + + def test_ext_does_not_change_the_output(self): + loaded = self._jmap( + _ext={ + "defects": ["DuplicateFromDefect"], + "resent": {"from": [{"name": "R", "email": "resender@e.co"}]}, + } + ) + assert compose_email(loaded) == compose_email(self._jmap()) + + def test_ext_resent_alone_emits_no_resent_headers(self): + """The asymmetry: parse produces ``_ext.resent``, compose ignores + it. Resent-* round-trips through ``headers``, not through here.""" + raw = compose_email( + self._jmap(_ext={"resent": {"from": [{"name": None, "email": "r@e.co"}]}}) + ) + assert b"Resent-From" not in raw + + def test_resent_headers_round_trip_through_headers(self): + """The path that does work, so the test above reads as a scope + boundary rather than a bug.""" + raw = compose_email( + self._jmap(headers=[{"name": "Resent-From", "value": "r@e.co"}]) + ) + assert b"Resent-From: r@e.co" in raw + + def test_ext_is_always_accepted(self): + """There is no flag to reject it. ``_ext`` cannot reach the + output, so refusing it protected nothing — a caller wanting to + assert strict RFC 8621 input checks ``"_ext" in data`` itself.""" + assert compose_email(self._jmap(_ext={"defects": []})) == compose_email( + self._jmap() + ) + + +class TestAddrSpecCheckedAsSupplied: + """A control character in an address is an error, not something to + quietly delete. + + ``is_valid_addr_spec`` promises a value is safe to place in a header + *as it stands*, the same promise ``is_valid_msg_id`` makes. Sanitizing + an address before validating it breaks that: the control character + disappears, what is left validates, and the composer emits a + recipient the caller never wrote. Caught by the round-trip fuzz + property ``test_no_address_appears_that_we_never_supplied``. + """ + + @pytest.mark.parametrize( + "addr", + [ + pytest.param("references@1&Q\x1a", id="c0-substitute"), + pytest.param("a@b.co\r\nBcc: evil@x.co", id="crlf-injection"), + pytest.param("a\x7f@b.co", id="del-in-local"), + pytest.param("a@b.co\u2028", id="line-separator"), + pytest.param("a@b.co\u0085", id="nel"), + ], + ) + def test_predicate_rejects_what_the_composer_would_strip(self, addr): + assert is_valid_addr_spec(addr) is False + + def test_composer_refuses_rather_than_cleaning(self): + """The failure mode this closes: the survivor of a strip is a + different recipient from the one supplied.""" + with pytest.raises(InvalidAddressError): + compose_email( + { + "from": [{"name": None, "email": "s@e.co"}], + "to": [{"name": "x", "email": "references@1&Q\x1a"}], + "subject": "s", + "sentAt": "2026-01-01T00:00:00+00:00", + "textBody": [{"content": "b"}], + } + ) + + def test_format_address_returns_empty_rather_than_cleaning(self): + assert format_address("x", "a@b.co\x1a") == "" + + def test_surrounding_whitespace_is_still_tolerated(self): + """Trimming the edges is normalization, not a content change.""" + assert format_address("x", " a@b.co ") == "x " + + def test_the_stripped_set_has_one_definition(self): + """The predicate and the composer's sanitizer must not drift — + that drift is exactly what let a cleaned address through.""" + from jmap_email.addresses import STRIPPED_HEADER_CHARS + from jmap_email.composer import _HEADER_INJECTION_CHARS + + assert set(_HEADER_INJECTION_CHARS) == STRIPPED_HEADER_CHARS + + +class TestAlreadyQuotedDisplayNameDetection: + """ "Already quoted" must mean a complete quoted-string. + + ``format_address`` skips re-quoting a display name that is already a + quoted-string. Testing that with ``startswith('"') and + endswith('"')`` alone accepts three things that are not one: a lone + ``"`` (one character satisfies both at once), ``"a"b"`` (closes + early), and ``"a\\"`` (ends on a backslash escaping its own closing + quote). Emitting any of them verbatim unbalances the header, and in a + mailbox-list the *next* entry's display name is then read as an + address. + + Same root cause as the unterminated local-part in + :func:`jmap_email.is_valid_addr_spec`, which is why both now go + through the same helper. + """ + + @staticmethod + def _jmap(to): + return { + "from": [{"name": None, "email": "s@e.co"}], + "to": to, + "subject": "s", + "sentAt": "2026-01-01T00:00:00+00:00", + "textBody": [{"content": "b"}], + } + + @pytest.mark.parametrize( + "name", + [ + pytest.param('"', id="lone-quote"), + pytest.param('"a"b"', id="closes-early"), + pytest.param('"a\\"', id="trailing-escape"), + ], + ) + def test_incomplete_quoted_string_is_escaped_not_trusted(self, name): + formatted = format_address(name, "a@b.co") + # Whatever we emit must re-parse as exactly one mailbox, with the + # address we supplied. + pairs = email.utils.getaddresses([formatted]) + assert [addr for _n, addr in pairs] == ["a@b.co"] + + @pytest.mark.parametrize( + "name", + [ + pytest.param('"ok"', id="complete"), + pytest.param('""', id="empty-quoted-string"), + pytest.param('"a\\\\"', id="escaped-backslash"), + ], + ) + def test_complete_quoted_string_is_left_alone(self, name): + """No double-quoting: a name that is already a well-formed + quoted-string passes through untouched.""" + assert format_address(name, "a@b.co") == f"{name} " + + def test_display_name_cannot_become_a_recipient(self): + """The end-to-end failure this closes: an unbalanced quote in one + entry let the next entry's display name be read as an address, + and both real recipients vanished.""" + raw = compose_email( + self._jmap( + [ + {"name": '"', "email": '"a b"@c.co'}, + {"name": ", evil@x.co", "email": '"a b"@c.co'}, + ] + ) + ) + recovered = {a["email"] for a in (parse_email(raw).get("to") or [])} + assert recovered == {'"a b"@c.co'} + assert "evil@x.co" not in recovered + + +class TestAddrSpecRejectsRfc5322Specials: + """Specials outside a quoted-string / domain-literal are not one mailbox. + + RFC 5322 §3.2.3 lists ``( ) < > [ ] : ; @ \\ , . "`` as specials. Every + one except ``.`` ends whatever token a reader is mid-way through, so a + value carrying one unquoted means different things to different + parsers — which is the whole failure mode this predicate exists to + prevent. Measured against ``getaddresses``: + + * ``a(b@c.co, victim@x.co`` -> a comment swallows the rest, **zero** + addresses recovered, both recipients silently gone. + * ``a:b@c.co`` -> read as the group ``a`` containing ``b@c.co``, so + the address a reader sees is not the one we were handed. + """ + + @pytest.mark.parametrize( + "char", ["(", ")", "<", ">", "[", "]", ":", ";", "@", "\\", ",", '"'] + ) + def test_special_in_unquoted_local_part_is_rejected(self, char): + assert is_valid_addr_spec(f"a{char}b@c.co") is False + + @pytest.mark.parametrize( + "char", ["(", ")", "<", ">", "[", "]", ":", ";", "\\", ","] + ) + def test_special_in_domain_is_rejected(self, char): + assert is_valid_addr_spec(f"a@b{char}c.co") is False + + def test_dot_is_not_rejected(self): + """``.`` is the dot-atom separator, not a token terminator.""" + assert is_valid_addr_spec("a.b@c.d.co") is True + + @pytest.mark.parametrize( + "addr", + [ + pytest.param("a@[192.168.1.1]", id="ipv4-literal"), + pytest.param("a@[IPv6:::1]", id="ipv6-literal"), + pytest.param("a@[]", id="empty-literal"), + ], + ) + def test_domain_literal_keeps_its_brackets_and_colons(self, addr): + """A domain-literal is the one place ``[``, ``]`` and ``:`` are + structural rather than stray, so the check has to know the form.""" + assert is_valid_addr_spec(addr) is True + + @pytest.mark.parametrize( + "addr", + [ + pytest.param("a@[1.2.3.4", id="unclosed-literal"), + pytest.param("a@[a[b]", id="nested-open-bracket"), + pytest.param("a@[a\\b]", id="escape-inside-literal"), + # Legal dtext, rejected on the one-mailbox invariant: a + # reader blind to literal brackets cuts a mailbox-list at + # the comma and opens a comment at the paren, so + # ``getaddresses`` recovers zero mailboxes from either next + # to a second recipient. + pytest.param("a@[a,b]", id="comma-inside-literal"), + pytest.param("a@[a(b]", id="open-paren-inside-literal"), + pytest.param("a@[a)b]", id="close-paren-inside-literal"), + ], + ) + def test_malformed_domain_literal_is_rejected(self, addr): + assert is_valid_addr_spec(addr) is False + + @pytest.mark.parametrize( + "addr", + [ + pytest.param('"a(b"@c.co', id="paren-quoted"), + pytest.param('"a:b"@c.co', id="colon-quoted"), + pytest.param('"a,b"@c.co', id="comma-quoted"), + pytest.param('"a b"@c.co', id="space-quoted"), + ], + ) + def test_quoting_is_what_makes_a_special_safe(self, addr): + """Inside a quoted-string the same characters are data, and the + quoting is what keeps the value one mailbox.""" + assert is_valid_addr_spec(addr) is True + + @pytest.mark.parametrize( + "addr", + [ + pytest.param("plain@example.com", id="plain"), + pytest.param("a.b+tag@sub.example.co.uk", id="dots-and-plus"), + pytest.param("!#$%&'*+-/=?^_`{|}~@example.com", id="all-atext"), + pytest.param("a@localhost", id="no-dot-domain"), + pytest.param("a@b-c.example", id="hyphen-domain"), + pytest.param("josé@exemplé.fr", id="rfc6531"), + ], + ) + def test_real_addresses_are_unaffected(self, addr): + assert is_valid_addr_spec(addr) is True + + def test_an_accepted_address_is_exactly_one_mailbox(self): + """The invariant behind the predicate, checked against the stdlib + rather than restated.""" + for addr in [ + "plain@example.com", + '"a,b"@c.co', + "a.b+tag@sub.example.co.uk", + ]: + assert is_valid_addr_spec(addr) + pairs = email.utils.getaddresses([f"{addr}, victim@x.co"]) + assert len(pairs) == 2, f"{addr!r} did not stay one mailbox" + assert pairs[-1][1] == "victim@x.co" + + +class TestAttachmentFailuresPropagate: + """A bad attachment raises; it is never silently dropped. + + The wrappers used to filter falsy parts and fall back to an unwrapped + body "if every attachment fails to build" — unreachable, since + ``_create_attachment_part`` raises on every bad-input branch. Pinning + the real contract so the dead code cannot come back as a behaviour + change: silently losing an attachment is invisible data loss for the + sender, which is the one outcome this composer refuses. + """ + + @staticmethod + def _jmap(attachments): + return { + "from": [{"name": None, "email": "s@e.co"}], + "to": [{"name": None, "email": "r@e.co"}], + "subject": "s", + "sentAt": "2026-01-01T00:00:00+00:00", + "textBody": [{"content": "b"}], + "attachments": attachments, + } + + GOOD = {"content": b"ok", "type": "text/plain", "name": "a.txt"} + + @pytest.mark.parametrize("disposition", ["attachment", "inline"]) + def test_a_single_bad_attachment_fails_the_whole_compose(self, disposition): + bad = {"type": "text/plain", "name": "b.txt", "disposition": disposition} + if disposition == "inline": + bad["cid"] = "c1" + with pytest.raises(AttachmentError): + compose_email(self._jmap([bad])) + + @pytest.mark.parametrize("disposition", ["attachment", "inline"]) + def test_a_bad_one_beside_a_good_one_still_raises(self, disposition): + """The filter that used to sit here would have dropped the bad + part and shipped the message with only the good one.""" + bad = {"type": "text/plain", "name": "b.txt", "disposition": disposition} + good = dict(self.GOOD, disposition=disposition) + if disposition == "inline": + bad["cid"] = "c1" + good["cid"] = "c2" + with pytest.raises(AttachmentError): + compose_email(self._jmap([good, bad])) + + def test_good_attachments_are_all_present(self): + """The other half: nothing is lost on the success path.""" + raw = compose_email( + self._jmap( + [ + {"content": b"one", "type": "text/plain", "name": "one.txt"}, + {"content": b"two", "type": "text/plain", "name": "two.txt"}, + ] + ) + ) + parsed = parse_email(raw) + assert {a["name"] for a in parsed["attachments"]} == {"one.txt", "two.txt"} + + +class TestAdversarialComplexityAndShapes: + """Attacker-supplied input must cost time proportional to its size. + + Every entry here is a measured finding, not a hypothetical. The bounds + are ~100x the fixed cost so they catch a change in the exponent + without being flaky about machine speed. + """ + + def test_display_name_angle_run_is_not_quadratic(self): + """``_ANGLE_ADDR_RE`` written ``<[^<>]*@[^<>]*>`` lets both halves + match ``@``, so a ``<`` followed by a run of ``@`` with no closing + ``>`` made the engine try every split point. Reachable straight + from a ``From`` display name: one 96 KiB message cost ~43s.""" + payload = '"<' + "@" * 90000 + '" ' + raw = ( + f"From: {payload}\r\nTo: x@y.co\r\nSubject: s\r\n" + "Date: Thu, 01 Jan 2026 00:00:00 +0000\r\n\r\nbody\r\n" + ).encode() + start = time.perf_counter() + parse_email(raw) + assert time.perf_counter() - start < 5.0 + + @pytest.mark.parametrize( + ("char", "what"), + [ + pytest.param("[", "markdown link text", id="bracket-run"), + pytest.param("<", "markdown autolink", id="angle-run"), + pytest.param("a", "autolink local-part", id="no-at-run"), + ], + ) + def test_preview_patterns_are_not_quadratic(self, char, what): + """The markdown link and autolink patterns had unbounded runs, so + a body of one repeated character rescanned to end of head from + every start position. At ``max_chars=65536`` that was 65s for + ``[`` and 114s for ``<``.""" + start = time.perf_counter() + preview_text(char * 200000, max_chars=65536) + assert time.perf_counter() - start < 10.0, what + + @pytest.mark.parametrize( + "jmap_data", + [ + pytest.param({"to": "not-a-list"}, id="address-list-is-a-string"), + pytest.param({"to": [None, 1, "x"]}, id="address-entries-not-dicts"), + pytest.param({"cc": [None]}, id="cc-entry-none"), + pytest.param({"attachments": "not-a-list"}, id="attachments-is-a-string"), + pytest.param({"attachments": [None]}, id="attachment-entry-none"), + pytest.param({"attachments": ["x"]}, id="attachment-entry-string"), + ], + ) + def test_malformed_shapes_raise_composeerror_not_attributeerror(self, jmap_data): + """``.get`` on a non-dict raised ``AttributeError``, which the + broad handler wrapped — but only after logging a full traceback. + A caller sending ``{"to": "x"}`` in a loop could flood the logs, + drowning real errors. The shape check now happens before the + access.""" + base = { + "from": [{"name": None, "email": "s@e.co"}], + "to": [{"name": None, "email": "r@e.co"}], + "subject": "s", + "sentAt": "2026-01-01T00:00:00+00:00", + "textBody": [{"content": "b"}], + } + try: + compose_email({**base, **jmap_data}) + except ComposeError: + pass # the documented failure mode + except Exception as exc: + pytest.fail(f"escaped as {type(exc).__name__}: {exc}") + + def test_format_address_list_tolerates_junk_entries(self): + """Public helper, shape comes from caller JSON.""" + assert format_address_list("not-a-list") == "" + assert format_address_list([None, 1, {"email": "a@b.co"}]) == "a@b.co" diff --git a/src/jmap-email/tests/test_composer_fuzz.py b/src/jmap-email/tests/test_composer_fuzz.py index c5fb1115..5c6b2405 100644 --- a/src/jmap-email/tests/test_composer_fuzz.py +++ b/src/jmap-email/tests/test_composer_fuzz.py @@ -8,7 +8,7 @@ ComposeError — never an unwrapped stdlib exception, never a crash, never a malformed output. Input paths covered: - - compose_email(jmap_data, in_reply_to, prepend_headers, keep_bcc) + - compose_email(jmap_data, in_reply_to, prepend_headers, emit_bcc) ↳ jmap_data fields: from, to, cc, bcc, subject, date, messageId, references, textBody, htmlBody, attachments, headers ↳ in_reply_to: arbitrary string @@ -30,9 +30,10 @@ from email import policy from email.parser import BytesParser import pytest -from hypothesis import HealthCheck, Phase, given, settings +from hypothesis import HealthCheck, given, settings from hypothesis import strategies as st +from jmap_email import ComposeOptions from jmap_email.composer import ( ComposeError, _normalize_date, @@ -48,7 +49,13 @@ FUZZ_SETTINGS = { "max_examples": int(os.environ.get("FUZZ_EXAMPLES", "2000")), "deadline": None, "suppress_health_check": [HealthCheck.too_slow, HealthCheck.data_too_large], - "phases": [Phase.generate, Phase.target], + # Phases are Hypothesis's defaults on purpose. ``shrink`` and + # ``explain`` cost nothing on a green run — they only engage once a + # failure exists, which is exactly when you want a minimal example + # rather than the raw generated blob. ``reuse`` replays a stored + # failure until it is fixed, which is what makes an intermittent + # find reproducible; it needs ``.hypothesis`` to survive the + # container, so compose mounts it. } @@ -236,7 +243,7 @@ class TestComposeEmailFuzz: @given(jmap=jmap_dict) @settings(**FUZZ_SETTINGS) def test_compose_drops_bcc_by_default(self, jmap): - """RFC 5322 §3.6.3 contract: Bcc never leaks unless keep_bcc=True.""" + """RFC 5322 §3.6.3 contract: Bcc never leaks unless emit_bcc=True.""" try: raw = compose_email(jmap) except ComposeError: @@ -271,19 +278,19 @@ class TestComposeEmailFuzz: for reserved in ("From", "To", "Subject", "Date"): assert len(parsed.get_all(reserved) or []) <= 1 - @given(jmap=jmap_dict, keep_bcc=st.booleans()) + @given(jmap=jmap_dict, emit_bcc=st.booleans()) @settings(**FUZZ_SETTINGS) - def test_compose_keep_bcc_flag_is_honored(self, jmap, keep_bcc): - """keep_bcc=True surfaces Bcc; False drops it. This is the contract + def test_compose_emit_bcc_flag_is_honored(self, jmap, emit_bcc): + """emit_bcc=True surfaces Bcc; False drops it. This is the contract PST import relies on.""" try: - raw = compose_email(jmap, keep_bcc=keep_bcc) + raw = compose_email(jmap, options=ComposeOptions(emit_bcc=emit_bcc)) except ComposeError: return parsed = BytesParser(policy=policy.default).parsebytes(raw) - if not keep_bcc: + if not emit_bcc: assert parsed["Bcc"] is None - # When keep_bcc=True, Bcc may or may not appear depending on whether + # When emit_bcc=True, Bcc may or may not appear depending on whether # the input had any bcc entries with a non-empty email. Don't assert # presence — just that the flag is honored on the no-leak side. @@ -388,7 +395,7 @@ class TestEndToEndPathFuzz: "from": contact_dict, "to": contact_list, "cc": contact_list, - "bcc": contact_list, # PST import preserves Bcc via keep_bcc=True + "bcc": contact_list, # PST import preserves Bcc via emit_bcc=True "subject": chaotic_text, "sentAt": chaotic_date, "messageId": st.one_of(st.none(), chaotic_text), @@ -402,10 +409,18 @@ class TestEndToEndPathFuzz: @given(jmap=pst_jmap) @settings(**FUZZ_SETTINGS) - def test_pst_import_path_with_keep_bcc(self, jmap): - """PST import → reconstruct_eml → compose_email(keep_bcc=True).""" + def test_pst_import_path_with_emit_bcc(self, jmap): + """PST import → reconstruct_eml → compose_email(...). + + Uses the same bundle the archive caller passes + (``ARCHIVE_COMPOSE_OPTIONS``) so the fuzzing covers the IDNA + normalization branch the real path takes, not just Bcc retention. + """ try: - raw = compose_email(jmap, keep_bcc=True) + raw = compose_email( + jmap, + options=ComposeOptions(idna_encode_domains=True, emit_bcc=True), + ) except ComposeError: return _assert_wire_format_invariants(raw) diff --git a/src/jmap-email/tests/test_filenames.py b/src/jmap-email/tests/test_filenames.py new file mode 100644 index 00000000..6be03b0d --- /dev/null +++ b/src/jmap-email/tests/test_filenames.py @@ -0,0 +1,162 @@ +"""Tests for :func:`jmap_email.sanitize_filename`. + +``parse_email`` applies this to every part name it reports; it is public +so consumers can apply it to names that never went through the parser. +Naming a *nameless* part is not covered here — such a part reports +``name: null`` per RFC 8621 and what to display instead is consumer +policy, deliberately outside this library. +""" + +import unicodedata + +import pytest + +from jmap_email import sanitize_filename + + +class TestSanitizeFilename: + """Contract of ``sanitize_filename``.""" + + def test_plain_name_untouched(self): + assert sanitize_filename("report.pdf") == "report.pdf" + + @pytest.mark.parametrize( + ("raw", "expected"), + [ + pytest.param("../../etc/passwd", "passwd", id="posix-traversal"), + pytest.param("..\\..\\boot.ini", "boot.ini", id="windows-traversal"), + pytest.param("/var/tmp/evil.sh", "evil.sh", id="absolute-posix"), + pytest.param("C:\\Users\\x\\evil.exe", "evil.exe", id="absolute-windows"), + ], + ) + def test_strips_path_components(self, raw, expected): + assert sanitize_filename(raw) == expected + + def test_strips_control_characters(self): + assert sanitize_filename("inv\r\noice\x00.pdf") == "invoice.pdf" + + @pytest.mark.parametrize( + ("raw", "expected"), + [ + # The classic attachment spoof: U+202E (right-to-left + # override) makes "annexegpj.exe" render as + # "annexe.exe.jpg" — an image to the user, an executable to + # the OS. + pytest.param("annexe\u202egpj.exe", "annexegpj.exe", id="bidi-override"), + pytest.param("a\u200bb.pdf", "ab.pdf", id="zero-width-space"), + pytest.param("\ufeffreport.pdf", "report.pdf", id="bom"), + pytest.param("a\u2028b.pdf", "ab.pdf", id="line-separator"), + pytest.param("a\u2029b.pdf", "ab.pdf", id="paragraph-separator"), + pytest.param("a\u0085b.pdf", "ab.pdf", id="c1-next-line"), + pytest.param("a\u00adb.pdf", "ab.pdf", id="soft-hyphen"), + pytest.param("a\u200db.pdf", "ab.pdf", id="zero-width-joiner"), + ], + ) + def test_strips_invisible_characters(self, raw, expected): + """Anything invisible to the reader but meaningful to the OS goes.""" + assert sanitize_filename(raw) == expected + + def test_invisible_character_cannot_shield_framing(self): + """Regression: the strip used to run before invisibles were removed, + so a control character protected a leading ``..`` from it and the + parent-directory segment survived intact.""" + assert sanitize_filename("\x00..\x00") is None + assert sanitize_filename("\u202e..\u202e") is None + + @pytest.mark.parametrize( + ("raw", "expected"), + [ + # Fullwidth forms pass any ASCII-based check, then NFKC folds + # them to "/" and "." downstream. Normalizing first turns them + # into a real path, which the basename strip then eats. + pytest.param("．．／etc／passwd", "passwd", id="fullwidth-traversal"), + pytest.param("a／b.txt", "b.txt", id="fullwidth-solidus"), + # U+2026 folds to "..." — a traversal segment in disguise. + pytest.param("…", None, id="ellipsis-is-dots"), + # Fullwidth Latin canonicalizes rather than staying exotic. + pytest.param("ｆｉｌe.txt", "file.txt", id="fullwidth-latin"), + # NBSP folds to a plain space, which the end-strip then removes. + pytest.param(" report.pdf ", "report.pdf", id="nbsp-framing"), + ], + ) + def test_normalizes_before_sanitizing(self, raw, expected): + """Compatibility forms are folded first, so a downstream ``NFKC`` + cannot reintroduce a separator we already removed.""" + assert sanitize_filename(raw) == expected + + def test_output_is_nfkc_stable(self): + """Normalizing the result again must be a no-op.""" + for raw in ("．．／x", "……", "ｆ.txt", "réçu.pdf"): + out = sanitize_filename(raw) + if out is not None: + assert unicodedata.normalize("NFKC", out) == out + + @pytest.mark.parametrize( + ("raw", "expected"), + [ + # Windows silently drops trailing dots and spaces, so without + # this "x.exe " and "x.exe" name the same file there while + # looking different to any allowlist. + pytest.param("report.pdf ", "report.pdf", id="trailing-space"), + pytest.param("report.pdf.", "report.pdf", id="trailing-dot"), + pytest.param(" report.pdf ", "report.pdf", id="surrounding-space"), + pytest.param(" ", None, id="whitespace-only"), + ], + ) + def test_strips_surrounding_whitespace_and_dots(self, raw, expected): + assert sanitize_filename(raw) == expected + + def test_replaces_dangerous_characters(self): + assert sanitize_filename("ac:d.txt") == "a_b_c_d.txt" + + def test_ntfs_alternate_data_stream_is_defanged(self): + """``name.txt:payload`` addresses an NTFS stream, not a file.""" + assert sanitize_filename("report.txt:evil.exe") == "report.txt_evil.exe" + + def test_idempotent(self): + once = sanitize_filename('../we"ird\r\nname.tar.gz') + assert sanitize_filename(once) == once + + def test_truncates_preserving_extension(self): + name = sanitize_filename("a" * 300 + ".pdf") + assert len(name) == 255 + assert name.endswith(".pdf") + + def test_truncates_unreasonable_extension_flat(self): + # An "extension" longer than 10 chars is not worth preserving. + name = sanitize_filename("a" * 300 + "." + "b" * 20) + assert len(name) == 255 + + @pytest.mark.parametrize( + "raw", + [ + pytest.param(None, id="none"), + pytest.param("", id="empty"), + # Nothing recognizable survives sanitizing. + pytest.param("...", id="dots-only"), + pytest.param("\x00\x01", id="control-only"), + pytest.param("/", id="separator-only"), + ], + ) + def test_returns_none_when_there_is_no_name(self, raw): + # ``None``, never ``""``: the empty string is not a filename, and + # the RFC 8621 field this feeds is ``String | null``. + assert sanitize_filename(raw) is None + + @pytest.mark.parametrize("max_length", [0, -1, -255]) + def test_returns_none_when_no_room(self, max_length): + # Regression: a negative ``max_length`` used to reach + # ``filename[:max_length]`` and slice from the *end*, so + # ``("ab.pdf", -5)`` answered ``"a"``. + assert sanitize_filename("ab.pdf", max_length=max_length) is None + + def test_leading_dot_name_loses_dot(self): + # ``.gitignore``-style names lose the leading dot to the strip of + # dot/slash/quote framing characters. + assert sanitize_filename(".gitignore") == "gitignore" + + def test_honours_explicit_max_length(self): + # Callers whose storage caps names below 255 pass their own limit. + name = sanitize_filename("a" * 300 + ".pdf", max_length=64) + assert len(name) == 64 + assert name.endswith(".pdf") diff --git a/src/jmap-email/tests/test_filenames_fuzz.py b/src/jmap-email/tests/test_filenames_fuzz.py new file mode 100644 index 00000000..472cc5a9 --- /dev/null +++ b/src/jmap-email/tests/test_filenames_fuzz.py @@ -0,0 +1,215 @@ +""" +Fuzzing tests for the attachment-filename sanitizer. + +These tests use hypothesis for property-based testing to pin the +structural contract of :func:`jmap_email.sanitize_filename` on arbitrary +and adversarial input: it never raises, it never returns the empty +string, its output fits the length budget, and nothing that could +redirect a write or misrepresent the name — path separators, traversal +segments, invisible characters — survives it. + +Run with: pytest -m fuzz tests/test_filenames_fuzz.py +Or: make fuzz-jmap-email +""" + +import os +import unicodedata + +import pytest +from hypothesis import HealthCheck, given, settings +from hypothesis import strategies as st + +from jmap_email import sanitize_filename + +# Intensive fuzzing settings +FUZZ_SETTINGS = { + # Override for a deeper soak: FUZZ_EXAMPLES=100000 make fuzz-jmap-email + "max_examples": int(os.environ.get("FUZZ_EXAMPLES", "10000")), + "deadline": None, # No time limit per example + "suppress_health_check": [HealthCheck.too_slow, HealthCheck.data_too_large], + # Phases are Hypothesis's defaults on purpose. ``shrink`` and + # ``explain`` cost nothing on a green run — they only engage once a + # failure exists, which is exactly when you want a minimal example + # rather than the raw generated blob. ``reuse`` replays a stored + # failure until it is fixed, which is what makes an intermittent + # find reproducible; it needs ``.hypothesis`` to survive the + # container, so compose mounts it. +} + +# Fragments biased toward what the sanitizer reacts to, so the fuzz +# doesn't spend its whole budget on inert unicode: traversal segments in +# both separator dialects, drive letters, the characters a filesystem +# chokes on, control bytes, and the framing characters stripped from the +# ends. +_hostile_fragment = st.one_of( + st.text(max_size=20), + st.sampled_from( + [ + "../", + "..\\", + "..", + ".", + "/", + "\\", + "//", + "C:", + "C:\\", + "\\\\server\\share\\", + "/etc/passwd", + "%2e%2e%2f", + "\x00", + "\x01", + "\x1f", + "\x7f", + "\r", + "\n", + "\r\n", + "\t", + "\u0085", + "\u2028", + "\u2029", + "\u202e", + "\u200b", + "\u200d", + "\ufeff", + "\u00ad", + "\uff0f", # fullwidth solidus → "/" under NFKC + "\uff0e", # fullwidth full stop → "." under NFKC + "\uff3c", # fullwidth reverse solidus → "\\" under NFKC + "\u2026", # horizontal ellipsis → "..." under NFKC + "\u00a0", # no-break space → " " under NFKC + "\u2044", # fraction slash + "CON", + "NUL", + "COM1", + "LPT1", + "x\u0301", # combining acute — truncation must not orphan it + "<", + ">", + ":", + '"', + "|", + "?", + "*", + ".pdf", + ".tar.gz", + ".gitignore", + "réçu", + "\ud83d\ude00", + " ", + ] + ), +) + +hostile_names = st.lists(_hostile_fragment, max_size=30).map("".join) + +# Both separator dialects are stripped, on every platform — the wire +# doesn't tell us which OS produced the name. +SEPARATORS = ("/", "\\") + +# Everything invisible: controls, bidi/format characters, the Unicode +# line separators, lone surrogates. Removed outright rather than +# replaced — they have no display semantics but plenty of OS semantics. +INVISIBLE_CATEGORIES = frozenset({"Cc", "Cf", "Zl", "Zp", "Cs"}) + + +@pytest.mark.fuzz +class TestSanitizeFilenameFuzz: + """Structural contract of ``sanitize_filename`` under fuzzing.""" + + @settings(**FUZZ_SETTINGS) + @given(raw=st.one_of(st.text(max_size=1000), hostile_names, st.none())) + def test_never_raises_and_never_returns_empty(self, raw): + """Any input yields ``None`` or a genuinely non-empty name.""" + out = sanitize_filename(raw) + assert out is None or (isinstance(out, str) and out != "") + + @settings(**FUZZ_SETTINGS) + @given( + raw=st.one_of(st.text(max_size=1000), hostile_names), + max_length=st.integers(min_value=-10, max_value=300), + ) + def test_respects_max_length(self, raw, max_length): + """Output never exceeds the budget, whatever the budget is. + + Includes non-positive budgets, which must answer ``None`` rather + than slicing from the end of the string. + """ + out = sanitize_filename(raw, max_length=max_length) + assert out is None or len(out) <= max_length + + @settings(**FUZZ_SETTINGS) + @given(raw=st.one_of(st.text(max_size=1000), hostile_names)) + def test_output_cannot_redirect_a_write(self, raw): + """No separator, no traversal segment, no control character. + + These are the properties that make the result safe to join onto a + directory or hand to a storage backend. + """ + out = sanitize_filename(raw) + if out is None: + return + assert not any(sep in out for sep in SEPARATORS) + assert not any(unicodedata.category(c) in INVISIBLE_CATEGORIES for c in out) + # ``.`` / ``..`` name the current and parent directory; the strip + # of leading and trailing dots means neither can survive whole. + assert out not in {".", ".."} + assert not out.startswith(".") + + @settings(**FUZZ_SETTINGS) + @given(raw=st.one_of(st.text(max_size=1000), hostile_names)) + def test_normalizing_the_output_reintroduces_nothing(self, raw): + """The result is NFKC-stable, and stays safe after normalizing. + + Sanitize-then-normalize is the documented bypass class + (CVE-2025-52488): a fullwidth solidus survives an ASCII check and + folds to "/" later. Since we normalize first, the output must be + a fixed point — and re-checking the safety properties on the + normalized form must still hold. + """ + out = sanitize_filename(raw) + if out is None: + return + assert unicodedata.normalize("NFKC", out) == out + assert not any(sep in out for sep in SEPARATORS) + assert out not in {".", ".."} + + @settings(**FUZZ_SETTINGS) + @given( + raw=st.one_of(st.text(max_size=1000), hostile_names), + max_length=st.integers(min_value=1, max_value=300), + ) + def test_idempotent(self, raw, max_length): + """Sanitizing an already-sanitized name changes nothing. + + Consumers re-sanitize names that already went through the parser + (the backend does exactly this before storing one), so a second + pass must not erode the name. + """ + once = sanitize_filename(raw, max_length=max_length) + if once is None: + return + assert sanitize_filename(once, max_length=max_length) == once + + @settings(**FUZZ_SETTINGS) + @given( + stem=st.text( + alphabet=st.characters( + exclude_categories=("Cs", "Cc"), exclude_characters='<>:"|?*\\/.' + ), + min_size=1, + max_size=400, + ), + ext=st.sampled_from([".pdf", ".txt", ".tar.gz", ".jpeg", ".ics"]), + ) + def test_extension_survives_truncation(self, stem, ext): + """A recognizable extension is preserved when the name is cut. + + The point of the truncation branch: the recipient's OS still + opens the file with the right application. + """ + out = sanitize_filename(stem + ext, max_length=64) + assert out is not None + # ``.tar.gz`` is 7 chars, so the last suffix is what survives. + assert out.endswith(ext.rsplit(".", 1)[-1]) + assert len(out) <= 64 diff --git a/src/jmap-email/tests/test_helpers.py b/src/jmap-email/tests/test_helpers.py index 11db952f..14ac572b 100644 --- a/src/jmap-email/tests/test_helpers.py +++ b/src/jmap-email/tests/test_helpers.py @@ -7,7 +7,7 @@ defaults on absence, accepts only well-typed input) is what downstream callers rely on. """ -from datetime import datetime, timezone +from datetime import datetime, timedelta, timezone import pytest @@ -222,3 +222,146 @@ class TestBodyAccess: if __name__ == "__main__": pytest.main() + + +class TestAccessorsAreNullSafe: + """The module's documented guarantee: no accessor ever raises. + + Regression: ``find_header``/``find_headers``/``has_header``/ + ``body_text_joined`` called ``.get`` on their first argument + unguarded, so they raised ``AttributeError`` on ``None`` — which is + exactly what ``parse_email`` returns for unparseable input, making + ``find_header(parse_email(raw), "Subject")`` crash on the one input + the null-safe accessors exist to survive. + """ + + def test_header_accessors_on_none(self): + assert find_header(None, "Subject") == "" + assert find_headers(None, "Subject") == [] + assert has_header(None, "Subject") is False + + def test_body_accessors_on_none(self): + assert body_text_joined(None) == "" + assert body_part_text(None, {"partId": "1"}) == "" + + @pytest.mark.parametrize( + "value", + [ + pytest.param(None, id="none"), + pytest.param("", id="str"), + pytest.param(0, id="int"), + pytest.param([], id="list"), + pytest.param([{"name": "Subject"}], id="list-of-dicts"), + # Truthy non-iterables: caught by an ``isinstance`` guard, not + # by a falsiness check. ``first_address`` iterated these and + # raised TypeError. + pytest.param(5, id="truthy-int"), + pytest.param(True, id="truthy-bool"), + pytest.param(3.5, id="truthy-float"), + pytest.param("a@b.co", id="truthy-str"), + ], + ) + def test_no_accessor_raises_on_wrong_shape(self, value): + assert find_header(value, "Subject") == "" + assert find_headers(value, "Subject") == [] + assert has_header(value, "Subject") is False + assert body_text_joined(value) == "" + assert first_address(value) is None + assert first_msgid(value) == "" + + def test_body_text_joined_with_non_list_key(self): + assert body_text_joined({"textBody": "not-a-list"}) == "" + + +class TestMsgidChainIsHeaderSafe: + """``msgid_chain`` exists to be written straight into a header. + + That makes it the one accessor that must not hand back something + unwritable. It is the same reasoning ``is_valid_msg_id`` gives for + being strict: the caller keeps what it got and may put it somewhere + other than ``compose_email``. + """ + + @pytest.mark.parametrize( + ("ids", "reason"), + [ + pytest.param(["a@x\r\nBcc: evil@x.co"], "crlf", id="crlf-injection"), + pytest.param(["a@x\nBcc: evil@x.co"], "lf", id="lf-injection"), + pytest.param(["a b@x"], "folds mid-id", id="internal-space"), + pytest.param(["a\tb@x"], "folds mid-id", id="internal-tab"), + pytest.param(["ab@x"], "ambiguous token", id="nested-close"), + pytest.param(["a\x00b@x"], "control char", id="nul"), + ], + ) + def test_unwritable_entries_are_dropped(self, ids, reason): + assert msgid_chain(ids) == "", reason + + def test_good_entries_survive_a_dropped_neighbour(self): + assert msgid_chain(["a@x", "b\r\nc@x", "d@x"]) == " " + + @pytest.mark.parametrize( + ("ids", "expected"), + [ + pytest.param(["a@x"], "", id="bare"), + pytest.param([""], "", id="already-wrapped"), + pytest.param(["a@x", "b@x"], " ", id="chain"), + # obs-id-left: real Outlook/MAPI ids carry several "@". + pytest.param(["foo$@local@domain"], "", id="multi-at"), + # No "@" at all is malformed but harmless; a reassembly helper + # should not silently lose it. + pytest.param(["12345"], "<12345>", id="no-at"), + ], + ) + def test_legitimate_ids_round_trip(self, ids, expected): + assert msgid_chain(ids) == expected + + def test_output_can_be_written_into_a_header(self): + """The end-to-end promise: whatever comes out is emittable.""" + from email.parser import BytesParser + from email.policy import default as default_policy + + chain = msgid_chain(["a@x", "evil\r\nBcc: x@y.co", "b@x"]) + raw = f"References: {chain}\r\n\r\n".encode() + parsed = BytesParser(policy=default_policy).parsebytes(raw) + assert parsed["Bcc"] is None + assert len(parsed.keys()) == 1 + + +class TestSentAtToDatetimeIsAlwaysAware: + """The docstring promises tz-aware; a naive return breaks callers. + + ``datetime.fromisoformat`` yields a naive object for an input with no + offset, and mixing that into a comparison with an aware datetime + raises ``TypeError`` at the call site rather than here. + """ + + @pytest.mark.parametrize( + "value", + [ + pytest.param("2026-01-01T00:00:00+00:00", id="with-offset"), + pytest.param("2026-01-01T00:00:00", id="no-offset"), + pytest.param("2026-01-01", id="bare-date"), + pytest.param("2026-01-01T00:00:00+02:00", id="non-utc-offset"), + ], + ) + def test_result_is_always_comparable_to_an_aware_datetime(self, value): + result = sent_at_to_datetime(value) + assert result is not None + assert result.utcoffset() is not None + # The property that actually matters at the call site. + assert isinstance(result < datetime.now(timezone.utc), bool) + + def test_naive_datetime_input_is_stamped_utc(self): + assert sent_at_to_datetime(datetime(2026, 1, 1)) == datetime( + 2026, 1, 1, tzinfo=timezone.utc + ) + + def test_aware_datetime_input_keeps_its_offset(self): + aware = datetime(2026, 1, 1, tzinfo=timezone(timedelta(hours=2))) + assert sent_at_to_datetime(aware) == aware + assert sent_at_to_datetime(aware).utcoffset() == timedelta(hours=2) + + def test_unparseable_still_returns_none(self): + assert sent_at_to_datetime("not a date") is None + assert sent_at_to_datetime(None) is None diff --git a/src/jmap-email/tests/test_helpers_fuzz.py b/src/jmap-email/tests/test_helpers_fuzz.py new file mode 100644 index 00000000..8afe5e28 --- /dev/null +++ b/src/jmap-email/tests/test_helpers_fuzz.py @@ -0,0 +1,119 @@ +""" +Fuzzing tests for the null-safe shape accessors and the msg-id validator. + +:mod:`jmap_email.helpers` promises that every accessor "returns a +sensible default on absence; none of them ever raises". That is a +property, and it had no property test — four of the accessors raised +``AttributeError`` on ``None``, the very value :func:`parse_email` +returns for input it cannot parse. + +Run with: pytest -m fuzz tests/test_helpers_fuzz.py +Or: make fuzz-jmap-email +""" + +import os + +import pytest +from hypothesis import HealthCheck, given, settings +from hypothesis import strategies as st + +import jmap_email +from jmap_email import is_valid_msg_id + +FUZZ_SETTINGS = { + # Override for a deeper soak: FUZZ_EXAMPLES=100000 make fuzz-jmap-email + "max_examples": int(os.environ.get("FUZZ_EXAMPLES", "10000")), + "deadline": None, + "suppress_health_check": [HealthCheck.too_slow, HealthCheck.data_too_large], + # Phases are Hypothesis's defaults on purpose. ``shrink`` and + # ``explain`` cost nothing on a green run — they only engage once a + # failure exists, which is exactly when you want a minimal example + # rather than the raw generated blob. ``reuse`` replays a stored + # failure until it is fixed, which is what makes an intermittent + # find reproducible; it needs ``.hypothesis`` to survive the + # container, so compose mounts it. +} + +# Arbitrary junk in the first-argument position: the accessors are the +# library's answer to "stop writing `parsed.get(x) or []`", so they are +# exactly what a caller reaches for before checking anything. +junk = st.recursive( + st.none() + | st.booleans() + | st.integers() + | st.floats(allow_nan=True) + | st.text(max_size=20) + | st.binary(max_size=20), + lambda children: ( + st.lists(children, max_size=4) + | st.dictionaries(st.text(max_size=8), children, max_size=4) + ), + max_leaves=8, +) + +# Accessors taking (parsed_email, name) and (parsed_email,) respectively. +NAMED = ["find_header", "find_headers", "has_header"] +SHAPE = [ + "first_address", + "first_address_email", + "first_address_name", + "first_msgid", + "msgid_chain", + "sent_at_to_datetime", +] + + +@pytest.mark.fuzz +class TestHelpersNeverRaise: + """The documented guarantee, as a property.""" + + @settings(**FUZZ_SETTINGS) + @given(value=junk, name=st.text(max_size=12)) + def test_header_accessors(self, value, name): + for fn_name in NAMED: + getattr(jmap_email, fn_name)(value, name) + + @settings(**FUZZ_SETTINGS) + @given(value=junk) + def test_shape_accessors(self, value): + for fn_name in SHAPE: + getattr(jmap_email, fn_name)(value) + + @settings(**FUZZ_SETTINGS) + @given(value=junk, key=st.sampled_from(["textBody", "htmlBody", "nope"])) + def test_body_accessors(self, value, key): + jmap_email.body_text_joined(value, key) + jmap_email.body_part_text(value, value) + + @settings(**FUZZ_SETTINGS) + @given(value=junk) + def test_body_part_text_with_arbitrary_part(self, value): + jmap_email.body_part_text({"bodyValues": {"1": {"value": "x"}}}, value) + + +@pytest.mark.fuzz +class TestIsValidMsgIdFuzz: + """``True`` must mean "usable exactly as given".""" + + @settings(**FUZZ_SETTINGS) + @given(value=junk) + def test_never_raises_and_returns_bool(self, value): + assert isinstance(is_valid_msg_id(value), bool) + + @settings(**FUZZ_SETTINGS) + @given( + value=st.text(max_size=120) + | st.builds( + lambda a, b: f"<{a}@{b}>", st.text(max_size=30), st.text(max_size=30) + ) + ) + def test_accepted_ids_need_no_cleaning(self, value): + """A caller keeps the raw string it validated, so an accepted value + must already be free of anything the composer would strip on the + way out — otherwise ``True`` hands back an injection payload.""" + # pylint: disable=protected-access + from jmap_email.composer import _sanitize_header_value + + if is_valid_msg_id(value): + assert _sanitize_header_value(value) == value + assert "\r" not in value and "\n" not in value diff --git a/src/jmap-email/tests/test_message_fuzz.py b/src/jmap-email/tests/test_message_fuzz.py index c6c832a6..bbd3fc50 100644 --- a/src/jmap-email/tests/test_message_fuzz.py +++ b/src/jmap-email/tests/test_message_fuzz.py @@ -9,19 +9,26 @@ Or: make fuzz-back """ import base64 +import os import pytest -from hypothesis import HealthCheck, Phase, given, settings +from hypothesis import HealthCheck, given, settings from hypothesis import strategies as st from jmap_email.parser import parse_email # Intensive fuzzing settings FUZZ_SETTINGS = { - "max_examples": 10000, + "max_examples": int(os.environ.get("FUZZ_EXAMPLES", "10000")), "deadline": None, # No time limit per example "suppress_health_check": [HealthCheck.too_slow, HealthCheck.data_too_large], - "phases": [Phase.generate, Phase.target], # Skip shrinking for speed + # Phases are Hypothesis's defaults on purpose. ``shrink`` and + # ``explain`` cost nothing on a green run — they only engage once a + # failure exists, which is exactly when you want a minimal example + # rather than the raw generated blob. ``reuse`` replays a stored + # failure until it is fixed, which is what makes an intermittent + # find reproducible; it needs ``.hypothesis`` to survive the + # container, so compose mounts it. } diff --git a/src/jmap-email/tests/test_options.py b/src/jmap-email/tests/test_options.py index 09f9ac0c..d5d0504e 100644 --- a/src/jmap-email/tests/test_options.py +++ b/src/jmap-email/tests/test_options.py @@ -1,7 +1,7 @@ """Tests for the per-call :class:`ParseOptions` context. Pin the behavior that: -- Defaults reproduce the historical module-constant values. +- The default caps are the documented values. - ``ParseOptions`` is frozen (a returned dict cannot be mutated by a caller and have that leak across other call sites). - Custom ``options=`` actually changes parser behavior — both wider @@ -16,25 +16,19 @@ from dataclasses import FrozenInstanceError import pytest from jmap_email import DEFAULT_PARSE_OPTIONS, ParseOptions, parse_addresses, parse_email -from jmap_email.parser import ( - MAX_ADDRESS_LIST_BYTES, - MAX_HEADER_VALUE_BYTES, - MAX_MIME_NESTING_DEPTH, - MAX_MIME_PARTS, -) class TestParseOptionsShape: """The dataclass is the public contract.""" - def test_default_constructor_matches_module_constants(self): - """``ParseOptions()`` reproduces the values exposed as - ``MAX_*`` on :mod:`jmap_email.parser`.""" + def test_default_caps_are_the_documented_values(self): + """Literals, not a comparison against the same field: changing a + default has to be a deliberate act that trips this.""" defaults = ParseOptions() - assert defaults.max_mime_nesting_depth == MAX_MIME_NESTING_DEPTH - assert defaults.max_mime_parts == MAX_MIME_PARTS - assert defaults.max_header_value_bytes == MAX_HEADER_VALUE_BYTES - assert defaults.max_address_list_bytes == MAX_ADDRESS_LIST_BYTES + assert defaults.max_mime_nesting_depth == 100 # Postfix mime_nesting_limit + assert defaults.max_mime_parts == 1000 # Go multipartmaxparts + assert defaults.max_header_value_bytes == 102_400 # Postfix header_size_limit + assert defaults.max_address_list_bytes == 100_000 def test_default_preview_cap_is_the_rfc_ceiling(self): """``max_preview_chars`` defaults to 256, the RFC 8621 §4.1.4 @@ -90,7 +84,9 @@ class TestCustomOptionsOnParseEmail: c += _count(sub) return c - assert _count(parsed["bodyStructure"]) <= MAX_MIME_PARTS + 5 + assert _count(parsed["bodyStructure"]) <= ( + DEFAULT_PARSE_OPTIONS.max_mime_parts + 5 + ) def test_tighter_limits_truncate_earlier(self): """A 100-part cap truncates a 200-part input even though the @@ -128,24 +124,29 @@ class TestCustomOptionsOnParseEmail: # size; total is root + 1500. assert _count(parsed["bodyStructure"]) >= 1500 - def test_default_caps_truncate_header_value(self): - """A header value beyond the default 100 KB cap gets truncated.""" - huge = b"x" * (MAX_HEADER_VALUE_BYTES + 1000) + def test_default_cap_rejects_an_over_long_header_value(self): + """Rejected, not truncated: there is no safe cut point for an + arbitrary field, and a shortened one still looks well-formed.""" + cap = DEFAULT_PARSE_OPTIONS.max_header_value_bytes + huge = b"x" * (cap + 1000) raw = b"From: a@b.c\r\nTo: d@e.f\r\nX-Big: " + huge + b"\r\n\r\nbody\r\n" - parsed = parse_email(raw) - xbig = next(h for h in parsed["headers"] if h["name"].lower() == "x-big") - assert len(xbig["value"]) <= MAX_HEADER_VALUE_BYTES + assert parse_email(raw) is None - def test_tighter_header_cap_truncates_smaller(self): + def test_value_at_the_cap_still_parses(self): + """The cap is a ceiling, not an off-by-one rejection.""" + cap = DEFAULT_PARSE_OPTIONS.max_header_value_bytes + raw = b"From: a@b.c\r\nX-Big: " + (b"x" * (cap - 10)) + b"\r\n\r\nbody\r\n" + assert parse_email(raw) is not None + + def test_tighter_header_cap_rejects_smaller(self): raw = ( b"From: a@b.c\r\nTo: d@e.f\r\n" b"X-Med: " + (b"y" * 10000) + b"\r\n" b"\r\nbody\r\n" ) tight = ParseOptions(max_header_value_bytes=500) - parsed = parse_email(raw, options=tight) - xmed = next(h for h in parsed["headers"] if h["name"].lower() == "x-med") - assert len(xmed["value"]) <= 500 + assert parse_email(raw, options=tight) is None + assert parse_email(raw) is not None # unchanged under the default class TestPreviewCap: diff --git a/src/jmap-email/tests/test_parser.py b/src/jmap-email/tests/test_parser.py index 27b3120f..acfb3b8c 100644 --- a/src/jmap-email/tests/test_parser.py +++ b/src/jmap-email/tests/test_parser.py @@ -13,6 +13,7 @@ from email.header import Header import pytest +from jmap_email import DEFAULT_PARSE_OPTIONS from jmap_email.parser import ( _parse_message_content, decode_rfc2047_header, @@ -1507,8 +1508,11 @@ PDF content 2 assert content["attachments"][0]["name"] == "doc1.pdf" assert content["attachments"][1]["name"] == "doc2.pdf" - def test_infer_filename_unknown_type(self): - """Test filename inference for unknown content types.""" + def test_nameless_part_keeps_null_name(self): + """A part with no filename reports ``name`` as null, per the JMAP spec. + + Synthesizing a placeholder is the consumer's job, not the parser's. + """ raw_email = b"""From: sender@example.com To: recipient@example.com Subject: Unknown Type @@ -1523,7 +1527,6 @@ content message_obj = _stdlib_message(raw_email) content = _parse_message_content(message_obj) attachment = content["attachments"][0] - # Should return "unnamed" without extension for unknown types assert attachment["name"] is None def test_part_with_empty_body(self): @@ -1773,7 +1776,7 @@ Image content assert img_in_body is not None, "Inline image should be in textBody" def test_email_with_many_parts(self): - """Many MIME parts below ``MAX_MIME_PARTS=1000`` must surface + """Many MIME parts below ``max_mime_parts`` must surface all parts. (Cap behavior is tested separately in ``test_huge_part_count_does_not_explode``.) """ @@ -1801,11 +1804,11 @@ Content-Type: multipart/mixed; boundary="boundary" ) message_obj = _stdlib_message(raw_email) content = _parse_message_content(message_obj) - # Capped at ``MAX_MIME_PARTS=1000``; 50 stays well below the cap + # Capped at ``max_mime_parts``; 50 stays well below the cap assert len(content["textBody"]) == 50 def test_deeply_nested_multipart(self): - """Deeply nested multipart below ``MAX_MIME_NESTING_DEPTH=100`` + """Deeply nested multipart below ``max_mime_nesting_depth`` must surface all levels. (Bomb behavior is tested separately in ``test_deeply_nested_multipart_bomb_does_not_recursion_error``.) """ @@ -1844,7 +1847,7 @@ Content-Type: multipart/mixed; boundary="outer" ) message_obj = _stdlib_message(raw_email) content = _parse_message_content(message_obj) - # Capped at ``MAX_MIME_NESTING_DEPTH=100``; 10 levels stays well below + # Capped at ``max_mime_nesting_depth``; 10 levels stays well below assert len(content["textBody"]) >= 1 def test_header_with_control_characters_strips_nul_from_subject(self): @@ -2279,10 +2282,15 @@ class TestParserSecurityRegressions: be truncated gracefully, not blow up the worker. Modelled on the HackerOne "stack exhaustion in MIME multipart" - disclosure pattern. Postfix caps at ``mime_nesting_limit=100``. + disclosure pattern. + + Note this bomb reuses ONE boundary at every level. That shape + collapses in the stdlib rather than nesting, so it stays cheap + and is *not* what the caps reject — see ``TestBoundedParse`` for + the distinct-boundary case that is. """ # Build 500 levels of multipart/mixed nesting — well above our - # 100-level guard, well below CPython's 1000-frame limit but + # depth guard, well below CPython's 1000-frame limit but # close enough that an unguarded recursive walk would fail. depth = 500 body = b"Content-Type: text/plain\r\n\r\nDEEPEST_TEXT\r\n" @@ -2329,9 +2337,9 @@ class TestParserSecurityRegressions: def test_huge_address_list_is_bounded_in_time(self): """Dovecot CVE-2024-23184 / Postfix ``header_address_token_limit``: a hostile ``To:`` with 50_000 addresses must not allocate - unbounded memory or block the worker for more than a few - seconds. We cap the input bytes; the parsed list may be - smaller than the wire content, but parsing must complete.""" + unbounded memory or block the worker. The header is past + ``max_header_value_bytes``, so the message is refused — in + bounded time, which is what the CVE is about.""" import time as _time addrs = ", ".join(f"u{i}@example.com" for i in range(50_000)) @@ -2345,8 +2353,47 @@ class TestParserSecurityRegressions: start = _time.monotonic() parsed = parse_email(raw) elapsed = _time.monotonic() - start + assert parsed is None + assert elapsed < 10.0, f"rejecting 50k addresses took {elapsed:.2f}s" + + def test_address_list_under_the_header_cap_is_cut_at_a_separator(self): + """Between ``max_address_list_bytes`` and ``max_header_value_bytes`` + the list is still truncated — but at a mailbox separator, never + mid-token, or the cut manufactures an address nobody sent.""" + target = "ceo@bigcorp.example" + cap = DEFAULT_PARSE_OPTIONS.max_address_list_bytes + payload = "x" * (cap - len(target) - 2) + ", " + target + "-junk@evil.test" + assert len(payload) < DEFAULT_PARSE_OPTIONS.max_header_value_bytes + raw = b"From: " + payload.encode() + b"\r\nSubject: t\r\n\r\nbody\r\n" + parsed = parse_email(raw, extensions=True) assert parsed is not None - assert elapsed < 10.0, f"parsing 50k addresses took {elapsed:.2f}s" + got = [a["email"] for a in (parsed.get("from") or [])] + assert target not in got, f"forged {target} out of a byte-boundary cut" + assert "AddressListTruncatedDefect" in ( + (parsed.get("_ext") or {}).get("defects") or [] + ) + + def test_truncated_resent_list_is_marked_as_defect(self): + """The ``Resent-*`` projection is parsed after the base address + headers; a message whose *only* truncated list is ``Resent-To`` + must still carry ``AddressListTruncatedDefect`` — the dropped + recipients are just as invisible to the consumer.""" + target = "ceo@bigcorp.example" + cap = DEFAULT_PARSE_OPTIONS.max_address_list_bytes + payload = "x" * (cap - len(target) - 2) + ", " + target + "-junk@evil.test" + assert len(payload) < DEFAULT_PARSE_OPTIONS.max_header_value_bytes + raw = ( + b"From: a@b.com\r\n" + b"Resent-To: " + payload.encode() + b"\r\n" + b"Subject: t\r\n" + b"\r\n" + b"body\r\n" + ) + parsed = parse_email(raw, extensions=True) + assert parsed is not None + assert "AddressListTruncatedDefect" in ( + (parsed.get("_ext") or {}).get("defects") or [] + ) def test_huge_part_count_does_not_explode(self): """Go ``multipartmaxparts=1000`` analogue: a message with 2000 @@ -2381,11 +2428,11 @@ class TestParserSecurityRegressions: ) assert total <= 2 * 1000, f"part-count cap not enforced: total={total}" - def test_huge_single_header_value_is_truncated_not_quadratic(self): + def test_huge_single_header_value_is_refused_not_quadratic(self): """gh-136063: multiple quadratic sites in stdlib's - ``_header_value_parser``. We cap raw header values at - ``MAX_HEADER_VALUE_BYTES`` before decoding so the - worst-case parse time stays linear in the cap, not in the + ``_header_value_parser``. A header past + ``max_header_value_bytes`` is refused before decoding, so the + worst case is bounded by the cap rather than by the attacker-supplied length. Source: https://github.com/python/cpython/issues/136063 @@ -2405,8 +2452,8 @@ class TestParserSecurityRegressions: start = _time.monotonic() parsed = parse_email(raw) elapsed = _time.monotonic() - start - assert parsed is not None - assert elapsed < 10.0, f"5MB header parsed in {elapsed:.2f}s" + assert parsed is None + assert elapsed < 10.0, f"5MB header handled in {elapsed:.2f}s" def test_display_name_strips_crlf_injection(self): """Header-injection in the display name (Apache James @@ -3018,8 +3065,8 @@ class TestBufferOverflowShapeRegressions: """CVE-2000-0567 / CVE-2001-0125 (Outlook / OE GMT-date heap overflow): a giant Date field crashed Outlook's ``inetcomm.dll``. Python's ``parsedate_to_datetime`` is - memory-safe, but a 200 KB Date must not hang and must return - a fallback rather than raise. + memory-safe; a 200 KB Date is past ``max_header_value_bytes`` + and the message is refused, in bounded time and without raising. Source: https://docs.microsoft.com/security-updates/SecurityBulletins/2000/ms00-043 """ @@ -3037,8 +3084,8 @@ class TestBufferOverflowShapeRegressions: start = _time.monotonic() parsed = parse_email(raw) elapsed = _time.monotonic() - start - assert parsed is not None - assert elapsed < 5.0, f"long date parsed in {elapsed:.2f}s" + assert parsed is None + assert elapsed < 5.0, f"long date handled in {elapsed:.2f}s" def test_cert_1998_long_filename_in_content_disposition(self): """CERT CA-1998-10 (Netscape/Pine/OE): a long @@ -3068,7 +3115,7 @@ class TestBufferOverflowShapeRegressions: parsed = parse_email(raw) if parsed["attachments"]: name = parsed["attachments"][0]["name"] - # ``_sanitize_filename`` caps at 255 chars. + # ``sanitize_filename`` caps at 255 chars. assert len(name) <= 255, f"filename not truncated: len={len(name)}" def test_cve_2005_4348_fetchmail_zero_headers(self): @@ -3162,8 +3209,8 @@ class TestBufferOverflowShapeRegressions: start = _time.monotonic() parsed = parse_email(raw) elapsed = _time.monotonic() - start - assert parsed is not None - assert elapsed < 10.0, f"5MB encoded-word parsed in {elapsed:.2f}s" + assert parsed is None + assert elapsed < 10.0, f"5MB encoded-word handled in {elapsed:.2f}s" def test_content_type_duplicate_param_explosion(self): """Sendmail / Exchange (CVE-2005-1987 / CVE-2006-0027 class): @@ -3184,23 +3231,16 @@ class TestBufferOverflowShapeRegressions: def test_unfolded_one_megabyte_header_line(self): """Fetchmail ≤6.2.4 class: a single unfolded header line of - 50 MB triggered unbounded malloc. We cap raw header value at - ``MAX_HEADER_VALUE_BYTES``; the truncation must happen - before the value lands in the decoded dict.""" - from jmap_email.parser import MAX_HEADER_VALUE_BYTES + 50 MB triggered unbounded malloc. A value past + ``max_header_value_bytes`` is refused outright — no truncated + copy of it ever reaches the decoded dict.""" + cap = DEFAULT_PARSE_OPTIONS.max_header_value_bytes - big_line = b"X-Big: " + b"A" * (2 * MAX_HEADER_VALUE_BYTES) + b"\r\n" + big_line = b"X-Big: " + b"A" * (2 * cap) + b"\r\n" raw = ( b"From: a@b.com\r\n" + big_line + b"Subject: huge unfolded\r\n\r\nbody\r\n" ) - parsed = parse_email(raw) - assert parsed is not None - x_big = _header_all(parsed, "x-big") - if x_big and isinstance(x_big, list): - # Stored value must be capped at the configured limit. - assert len(x_big[0]) <= MAX_HEADER_VALUE_BYTES + 100, ( - f"raw header not truncated: len={len(x_big[0])}" - ) + assert parse_email(raw) is None class TestUnrecognisedMessageSubtypeRobustness: @@ -3981,12 +4021,12 @@ class TestParserPass4Regressions: def test_m22_body_structure_part_cap_caps_total_parts(self): """A pathological multipart with thousands of children must not explode memory. The body-structure walker bails at - ``MAX_MIME_PARTS``.""" - from jmap_email.parser import MAX_MIME_PARTS + ``max_mime_parts``.""" + cap = DEFAULT_PARSE_OPTIONS.max_mime_parts # Build a flat multipart/mixed with many text/plain leaves. parts = [] - n = MAX_MIME_PARTS + 50 + n = cap + 50 for i in range(n): parts.append(b"--B\r\nContent-Type: text/plain\r\n\r\nx%d\r\n" % i) raw = ( @@ -4007,14 +4047,12 @@ class TestParserPass4Regressions: return c total = _count(parsed["bodyStructure"]) - # The cap is enforced after ``MAX_MIME_PARTS`` leaves have been + # The cap is enforced after ``max_mime_parts`` leaves have been # collected; with the multipart root + 1000 leaves, the total # stays just above the cap and well below the input count. assert total < n, f"part cap not enforced: walked {total} of {n} input parts" # And not far above the cap itself (root + cap leaves + slack). - assert total <= MAX_MIME_PARTS + 5, ( - f"part cap exceeded by more than expected: {total}" - ) + assert total <= cap + 5, f"part cap exceeded by more than expected: {total}" # ----- L1: Content-ID stripping uses _strip_cfws + single bracket pair -- @@ -4210,3 +4248,370 @@ class TestPreviewCleaning: ) parsed = parse_email(raw) assert parsed["preview"] == "Ma reponse fraiche" + + +class TestUnclosedCommentAddressSpoof: + """An unclosed ``(`` must not turn a display name into the sender. + + ``getaddresses(strict=False)`` treats an unclosed comment as running + to end of input, so in ``victim@bank.com( `` the + comment eats the angle-addr and the splitter returns one tuple whose + *address* is what the sender typed in display-name position. + + That is CVE-2023-27043 by another route. + :func:`_pick_best_address` defends the multi-tuple form by preferring + the last plausible tuple; here the bogus tuple is the only one, so + the preference has nothing to choose between. A consumer keying + allow/deny, DMARC alignment or contact identity off the parsed + address would attribute the message to an address the sender merely + named and does not own. + """ + + @pytest.mark.parametrize( + "header", + [ + pytest.param("victim@bank.com( ", id="bare-display-addr"), + pytest.param("'victim@bank.com(' ", id="single-quoted"), + pytest.param( + "victim@bank.com(comment ", id="comment-text" + ), + pytest.param( + "victim@bank.com((( ", id="several-unclosed" + ), + ], + ) + def test_spoofed_sender_is_refused(self, header): + assert parse_address(header) == ("", "") + assert not parse_addresses(header) + + def test_one_unclosed_comment_truncates_the_rest_of_the_list(self): + """An unclosed comment costs every entry from it onwards, and no + earlier one. + + The comment runs to end of input, so the stdlib splitter has + already folded everything after it into the poisoned tuple's + display name — ``other@b.co`` is not a tuple we could keep even if + we wanted to. Entries *before* it were split normally and survive. + Recorded because it is the kind of partial result that looks like + a bug from either end: a caller comparing the returned count to + the comma count sees a mismatch, and the missing entries are the + tail rather than the malformed one alone. + """ + header = "good@a.co, victim@bank.com( , other@b.co" + assert parse_addresses(header) == [("", "good@a.co")] + + # With nothing before it, the whole header yields nothing. + assert not parse_addresses("victim@bank.com( , other@b.co") + + def test_parse_email_reports_no_sender_rather_than_the_wrong_one(self): + raw = ( + b"From: victim@bank.com( \r\n" + b"To: user@example.com\r\nSubject: hi\r\n" + b"Date: Thu, 01 Jan 2026 00:00:00 +0000\r\n\r\nbody\r\n" + ) + parsed = parse_email(raw) + assert parsed is not None + # Refusing beats naming the wrong mailbox. + assert not parsed.get("from") + + def test_lenient_mode_still_preserves_the_wire_bytes(self): + """Archive importers keep the original text; they just don't get + a fabricated addr-spec out of it.""" + name, addr = parse_address("victim@bank.com( ", lenient=True) + assert name == "" + assert addr == "victim@bank.com( " + + @pytest.mark.parametrize( + ("header", "expected"), + [ + # Parens inside a quoted-string are literal, not a comment. + pytest.param( + '"victim@evil.com(" ', + ("victim@evil.com(", "real@you.com"), + id="paren-inside-quotes", + ), + # A quoted display name may legitimately hold an angle-addr; + # with no unclosed comment there is nothing ambiguous. + pytest.param( + '"Bob " ', + ("Bob ", "bob@new.co"), + id="angle-addr-in-quoted-name", + ), + # Ordinary balanced comments keep working. + pytest.param( + "John (the boss) Doe ", + ("John Doe (the boss)", "john@example.com"), + id="balanced-comment", + ), + pytest.param( + "john@example.com (trailing comment)", + ("trailing comment", "john@example.com"), + id="trailing-comment", + ), + # The original CVE-2023-27043 shape still resolves to the + # angle-addr rather than the display name. + pytest.param( + '"a@b.co" ', + ("a@b.co", "real@you.com"), + id="cve-2023-27043", + ), + ], + ) + def test_legitimate_forms_are_untouched(self, header, expected): + assert parse_address(header) == expected + + +def _nested(depth, body_lines=4, line_len=2): + """A message nested ``depth`` multipart levels deep with a flat body. + + Each level gets a distinct boundary — reused boundaries collapse in + the stdlib and trip the separate body-smuggling defence. + """ + body = ( + b"Content-Type: text/plain\n\n" + (b"x" * (line_len - 1) + b"\n") * body_lines + ) + for i in range(depth): + tok = f"B{i}".encode() + body = ( + b'Content-Type: multipart/mixed; boundary="' + + tok + + b'"\n\n--' + + tok + + b"\n" + + body + + b"\n--" + + tok + + b"--\n" + ) + return b"From: a@b.com\nSubject: nested\n" + body + + +class TestFastSubFile: + """``_FastSubFile`` — the stdlib input buffer without the + per-ancestor scan on lines that cannot be a delimiter. + + ``BufferedSubFile.readline`` tests every body line against every + ancestor predicate, making the stdlib parse O(depth × lines): a + 10 MiB message measured 22.5 s at depth 100, against a 90 s worker + timeout. The body-tree depth cap could not help — it runs after the + parse, once the cost is already spent. + """ + + def test_probe_still_works_on_this_python(self): + """Guards the private-API coupling: if a Python upgrade moves + ``_input``, this fails instead of the buffer silently not being + used.""" + from jmap_email.parser import _FAST_SUBFILE_SUPPORTED, _probe_fast_subfile + + assert _FAST_SUBFILE_SUPPORTED is True + assert _probe_fast_subfile() is True + + @pytest.mark.parametrize( + "raw", + [ + b"From: a@b.com\nSubject: s\n\nbody\n", + b"From: a@b.com\r\nSubject: s\r\n\r\nbody\r\n", + b"From: a@b.com\nSubject: s\n\nbody", + b"From: a@b.com\nSubject: \xc3\xa9\n\nb\xffdy\n", + ], + ids=["lf", "crlf", "no-trailing-nl", "8bit"], + ) + def test_output_is_identical_to_message_from_bytes(self, raw): + from jmap_email.parser import _message_from_bytes + + got, want = _message_from_bytes(raw), _stdlib_message(raw) + assert got.as_bytes() == want.as_bytes() + assert [type(d).__name__ for d in got.defects] == [ + type(d).__name__ for d in want.defects + ] + + def test_delivery_status_blocks_survive(self): + """``_eofstack`` is not boundary-only. + + Inside a ``message/delivery-status`` the parser pushes + ``NLCRE.match``, for which a *blank* line is the false EOF. + Skipping the scan on it merges the per-message and per-recipient + blocks and turns ``Final-Recipient`` / ``Status`` into an + unparsed payload — the DSN stops being machine-readable. + """ + from jmap_email.parser import _message_from_bytes + + raw = ( + b"From: MAILER-DAEMON@relay.example\r\nTo: s@example.com\r\n" + b"Subject: Undelivered Mail\r\n" + b"Content-Type: multipart/report; report-type=delivery-status;\r\n" + b'\tboundary="RPT"\r\n\r\n' + b"--RPT\r\nContent-Type: text/plain\r\n\r\nfailed\r\n" + b"--RPT\r\nContent-Type: message/delivery-status\r\n\r\n" + b"Reporting-MTA: dns; relay.example\r\n\r\n" + b"Final-Recipient: rfc822; victim@example.org\r\n" + b"Action: failed\r\nStatus: 5.1.1\r\n\r\n" + b"--RPT--\r\n" + ) + + def blocks(msg): + dsn = [ + p + for p in msg.walk() + if p.get_content_type() == "message/delivery-status" + ] + assert dsn, "no delivery-status part" + return [ + sorted(k.lower() for k in sub.keys()) for sub in dsn[0].get_payload() + ] + + assert blocks(_message_from_bytes(raw)) == blocks(_stdlib_message(raw)) + assert any("final-recipient" in b for b in blocks(_message_from_bytes(raw))) + + def test_parse_cost_is_flat_in_depth(self): + """The point of the whole change: depth must stop multiplying + the per-line cost.""" + import time as _time + + shallow = _nested(2, body_lines=200_000, line_len=40) + deep = _nested(100, body_lines=200_000, line_len=40) + + start = _time.monotonic() + assert parse_email(shallow) is not None + shallow_cost = _time.monotonic() - start + + start = _time.monotonic() + assert parse_email(deep) is not None + deep_cost = _time.monotonic() - start + + # A ratio, not an absolute bound: both scale together on a slow + # runner. Without the fast path depth 100 costs several times + # depth 2; with it the two are comparable. + assert deep_cost < shallow_cost * 3, ( + f"depth 100 cost {deep_cost:.2f}s vs depth 2 {shallow_cost:.2f}s " + "— the O(depth) term is back" + ) + + def test_a_deep_message_with_a_large_body_is_cheap(self): + """The shape that used to take tens of seconds.""" + import time as _time + + raw = _nested(99, body_lines=300_000, line_len=21) + start = _time.monotonic() + assert parse_email(raw) is not None + elapsed = _time.monotonic() - start + assert elapsed < 2.0, f"depth 99 with a large body took {elapsed:.2f}s" + + def test_ordinary_nesting_still_parses(self): + parsed = parse_email(_nested(4, body_lines=10)) + assert parsed is not None + assert parsed["subject"] == "nested" + + def test_wide_but_shallow_still_parses(self): + """A ``multipart/digest`` with 200 sibling sub-parts.""" + parts = b"".join( + b'--OUT\r\nContent-Type: multipart/alternative; boundary="S' + + str(i).encode() + + b'"\r\n\r\n--S' + + str(i).encode() + + b"\r\nContent-Type: text/plain\r\n\r\nhi\r\n--S" + + str(i).encode() + + b"--\r\n" + for i in range(200) + ) + raw = ( + b"From: a@b.c\r\nSubject: digest\r\n" + b'Content-Type: multipart/digest; boundary="OUT"\r\n\r\n' + + parts + + b"--OUT--\r\n" + ) + parsed = parse_email(raw) + assert parsed is not None + assert parsed["subject"] == "digest" + + @pytest.mark.parametrize("blen", [69, 70, 71, 100, 200, 300, 1000]) + def test_over_long_boundaries_parse_as_the_stdlib_does(self, blen): + """RFC 2046 §5.1.1 caps a boundary at 70, but that binds the + *generator*: ``get_boundary`` applies no length check and matches + the delimiter literally, so a longer one is live everywhere.""" + b = ("B" * blen).encode() + raw = ( + b"From: a@b.com\r\nTo: c@d.com\r\nSubject: s\r\nMIME-Version: 1.0\r\n" + b'Content-Type: multipart/mixed; boundary="' + b + b'"\r\n\r\n' + b"--" + b + b"\r\nContent-Type: text/plain\r\n\r\nhello world\r\n" + b"--" + b + b"\r\nContent-Type: text/plain\r\n\r\nsecond part\r\n" + b"--" + b + b"--\r\n" + ) + parsed = parse_email(raw) + assert parsed is not None, f"boundary of {blen} bytes was rejected" + assert len(parsed["textBody"]) == len(_stdlib_message(raw).get_payload()) == 2 + + +class TestHeaderTruncationForgery: + """Truncating a header and *then* parsing it can manufacture an + address nobody sent. + + Pad a ``From`` so the byte cut lands after ``, ceo@bigcorp.example`` + in ``…, ceo@bigcorp.example-junk@attacker.test`` and the list parses + to exactly that address — which then becomes the stored sender and + the DKIM alignment domain. RFC 5322 §2.2.3 puts no limit on a field + ("may be indeterminately long"), so this is reachable with fully + conformant folding; a line-length rule does not help. + """ + + TARGET = "ceo@bigcorp.example" + + def _payload(self, cut, delta=0): + pad = "x" * (cut - len(self.TARGET) - 2 + delta) + return pad + ", " + self.TARGET + "-junk@attacker.test" + + def _from(self, payload): + raw = b"From: " + payload.encode() + b"\r\nSubject: t\r\n\r\nbody\r\n" + parsed = parse_email(raw) + return [a["email"] for a in ((parsed or {}).get("from") or [])] + + def test_unfolded_over_header_cap_is_refused(self): + cut = DEFAULT_PARSE_OPTIONS.max_header_value_bytes + raw = ( + b"From: " + self._payload(cut).encode() + b"\r\nSubject: t\r\n\r\nbody\r\n" + ) + assert parse_email(raw) is None + + def test_address_list_cut_never_forges_at_any_offset(self): + """One offset in a few hundred lands the cut exactly; sweep.""" + cut = DEFAULT_PARSE_OPTIONS.max_address_list_bytes + forged = [ + d + for d in range(-60, 61) + if self.TARGET in self._from(self._payload(cut, d)) + ] + assert not forged, f"forged at offsets {forged}" + + def test_folded_header_is_refused_too(self): + """Every physical line stays under RFC 5322's 998-octet limit, + so line length is no defence — the unfolded field is what counts. + """ + cut = DEFAULT_PARSE_OPTIONS.max_header_value_bytes + forged = [] + for delta in range(-60, 61): + pad = cut - len(self.TARGET) - 2 + delta + tokens, used = [], 0 + while used < pad: + chunk = min(900, pad - used) + tokens.append("x" * chunk) + used += chunk + field = "\r\n ".join(tokens) + ", " + self.TARGET + "-junk@attacker.test" + raw = b"From: " + field.encode() + b"\r\nSubject: t\r\n\r\nbody\r\n" + assert max(len(ln) for ln in raw.split(b"\r\n")) <= 998 + parsed = parse_email(raw) + if self.TARGET in [a["email"] for a in ((parsed or {}).get("from") or [])]: + forged.append(delta) + assert not forged, f"folded forgery at offsets {forged}" + + def test_cut_lands_only_on_a_top_level_separator(self): + """A comma inside a quoted-string, domain-literal or comment is + literal text; cutting there re-parses into a different address. + """ + from jmap_email.parser import _last_separator_index + + assert _last_separator_index("a@[1,2], b@c.com", 7) == -1 + assert _last_separator_index('"q,x"@d.com, b@c.com', 5) == -1 + assert _last_separator_index("(c,x) a@b.com, d@e.com", 5) == -1 + assert _last_separator_index('"a\\",b"@x.com, c@d.com', 8) == -1 + # ...but a real separator after the literal closes is found. + assert _last_separator_index("a@[1,2], b@c.com", 8) == 7 diff --git a/src/jmap-email/tests/test_preview.py b/src/jmap-email/tests/test_preview.py index 269a43ce..9e4992e6 100644 --- a/src/jmap-email/tests/test_preview.py +++ b/src/jmap-email/tests/test_preview.py @@ -6,6 +6,10 @@ BEFORE truncation, whitespace collapsed, length capped at 256 (the RFC 8621 §4.1.4 ceiling) unless the caller lowers ``max_chars``. """ +import time + +import pytest + from jmap_email import preview_text @@ -263,8 +267,6 @@ class TestPreviewHardening: def test_markup_bomb_is_bounded(self): # A body that is almost entirely markup never reaches the text budget; # the scan cap must still bound the work (regression for the DoS). - import time - start = time.perf_counter() preview_text("" * 500_000, max_scan_bytes=64 * 1024) assert (time.perf_counter() - start) < 1.0 # was seconds before the cap @@ -489,7 +491,41 @@ class TestPreviewParserQuirks: assert preview_text("a
b") == "a b" -if __name__ == "__main__": - import pytest +class TestPreviewComplexity: + """Wall-clock guards against quadratic matching returning. + Every line-anchored markdown pattern once used ``\\s`` for its leading + run. ``\\s`` matches ``\\n``, so under ``re.MULTILINE`` the match + rescanned the whole following whitespace run from every line start. + The head the patterns run on is bounded — but its size scales with + ``max_chars``, so a caller who raised that knob turned a 128 KiB body + of alternating spaces and newlines into >20s of matching. The bounds + below are ~100x the fixed cost, so they flag a regression in the + exponent without being flaky about machine speed. + """ + + # Alternating whitespace: one line start per two characters, which is + # what makes ``^\s*`` rescan. + BOMB = " \n" * 65000 + + @pytest.mark.parametrize("max_chars", [256, 4096, 16384, 65536]) + def test_widening_max_chars_stays_linear(self, max_chars): + start = time.perf_counter() + preview_text(self.BOMB, max_chars=max_chars) + assert time.perf_counter() - start < 5.0 + + def test_tab_newline_alternation(self): + start = time.perf_counter() + preview_text("\t\n" * 65000, max_chars=16384) + assert time.perf_counter() - start < 5.0 + + def test_at_run_without_a_dot(self): + """``[^@>\\s]+@[^@>\\s]+\\.[^@>\\s]+`` could split at every dot, + because the label class contained the dot it was splitting on.""" + start = time.perf_counter() + preview_text("<" + "a" * 60000 + "@" + "b" * 60000 + ">", max_chars=16384) + assert time.perf_counter() - start < 5.0 + + +if __name__ == "__main__": pytest.main() diff --git a/src/jmap-email/tests/test_preview_fuzz.py b/src/jmap-email/tests/test_preview_fuzz.py index 4c533ba7..fa3ccb14 100644 --- a/src/jmap-email/tests/test_preview_fuzz.py +++ b/src/jmap-email/tests/test_preview_fuzz.py @@ -10,18 +10,26 @@ Run with: pytest -m fuzz tests/test_preview_fuzz.py Or: make fuzz-jmap-email """ +import os + import pytest -from hypothesis import HealthCheck, Phase, given, settings +from hypothesis import HealthCheck, given, settings from hypothesis import strategies as st from jmap_email import preview_text # Intensive fuzzing settings FUZZ_SETTINGS = { - "max_examples": 10000, + "max_examples": int(os.environ.get("FUZZ_EXAMPLES", "10000")), "deadline": None, # No time limit per example "suppress_health_check": [HealthCheck.too_slow, HealthCheck.data_too_large], - "phases": [Phase.generate, Phase.target], # Skip shrinking for speed + # Phases are Hypothesis's defaults on purpose. ``shrink`` and + # ``explain`` cost nothing on a green run — they only engage once a + # failure exists, which is exactly when you want a minimal example + # rather than the raw generated blob. ``reuse`` replays a stored + # failure until it is fixed, which is what makes an intermittent + # find reproducible; it needs ``.hypothesis`` to survive the + # container, so compose mounts it. } # Fragments biased toward what the cleaning pipeline reacts to, so the diff --git a/src/jmap-email/tests/test_roundtrip_fuzz.py b/src/jmap-email/tests/test_roundtrip_fuzz.py new file mode 100644 index 00000000..7f66e306 --- /dev/null +++ b/src/jmap-email/tests/test_roundtrip_fuzz.py @@ -0,0 +1,245 @@ +""" +Fuzzing the parse/compose seam. + +The suites next door fuzz each direction alone: ``test_message_fuzz`` +feeds junk to the parser, ``test_composer_fuzz`` checks the composer's +output is wire-legal. Neither exercises the *seam* — and the seam is +where a mail system lives, because every reply, forward and autoreply is +a parse followed by a compose of something an attacker wrote. + +The properties here are about containment across that round trip: no +value carried through may turn into a header, a recipient, or a MIME +part that the input did not already contain. + +Run with: pytest -m fuzz tests/test_roundtrip_fuzz.py +Or: make fuzz-jmap-email +""" + +import email +import os + +import pytest +from hypothesis import HealthCheck, given, settings +from hypothesis import strategies as st + +from jmap_email import ComposeError, compose_email, is_valid_addr_spec, parse_email + +FUZZ_SETTINGS = { + # Override for a deeper soak: FUZZ_EXAMPLES=100000 make fuzz-jmap-email + "max_examples": int(os.environ.get("FUZZ_EXAMPLES", "10000")), + "deadline": None, + "suppress_health_check": [HealthCheck.too_slow, HealthCheck.data_too_large], + # Phases are Hypothesis's defaults on purpose. ``shrink`` and + # ``explain`` cost nothing on a green run — they only engage once a + # failure exists, which is exactly when you want a minimal example + # rather than the raw generated blob. ``reuse`` replays a stored + # failure until it is fixed, which is what makes an intermittent + # find reproducible; it needs ``.hypothesis`` to survive the + # container, so compose mounts it. +} + +# Text biased toward characters that mean something structural in a +# header: the separators, the delimiters, the line terminators, and the +# encoded-word syntax that smuggles all of them past a naive check. +_hostile_text = st.one_of( + st.text(max_size=40), + st.sampled_from( + [ + "\r\n", + "\n", + "\r", + "\r\n ", + "\r\nBcc: evil@x.co", + "\r\n\r\n", + ",", + ", evil@x.co", + ";", + "<", + ">", + '"', + "\\", + "@", + ":", + "\x00", + "\x1b", + "", + " ", + "=?utf-8?B?ZXZpbEB4LmNv?=", + "=?utf-8?q?a=0d=0aBcc:_e@x.co?=", + "--boundary", + "\r\n--boundary", + "Content-Type: text/html", + "é", + " ", + ] + ), +) + +_addr = st.builds( + lambda a, b: f"{a}@{b}", + st.text(max_size=12), + st.text(max_size=12), +) | st.sampled_from( + [ + "a@b.co", + "a@b.co, evil@x.co", + "a b@c.co", + "@b.co", + "a@", + '"a b"@c.co', + "é@ü.co", + ] +) + +_mailbox = st.builds( + lambda n, e: {"name": n, "email": e}, st.one_of(st.none(), _hostile_text), _addr +) + +jmap_email_dict = st.builds( + lambda frm, to, cc, subject, body: { + "from": [frm], + "to": to, + "cc": cc, + "subject": subject, + "sentAt": "2026-06-08T12:00:00+00:00", + "textBody": [{"partId": "1", "type": "text/plain", "content": body}], + }, + _mailbox, + st.lists(_mailbox, max_size=3), + st.lists(_mailbox, max_size=2), + _hostile_text, + _hostile_text, +) + + +def _compose(jmap): + try: + return compose_email(jmap) + except ComposeError: + return None + + +def _separator_commas(value: str) -> int: + """Count commas that actually separate mailboxes. + + A comma inside a quoted display name is data, not a separator. + """ + count = 0 + in_quotes = False + escaped = False + for ch in value: + if escaped: + escaped = False + elif ch == "\\": + escaped = True + elif ch == '"': + in_quotes = not in_quotes + elif ch == "," and not in_quotes: + count += 1 + return count + + +@pytest.mark.fuzz +class TestRoundTripContainment: + """Nothing may gain structure by passing through compose then parse.""" + + @settings(**FUZZ_SETTINGS) + @given(jmap=jmap_email_dict) + def test_recipient_count_never_grows(self, jmap): + """The wire must not carry more mailboxes than we were given. + + A single ``email`` containing a comma used to become two + recipients: the mailbox-list is built by joining on commas, so a + value carrying one is address injection performed by the library + on its caller's behalf. + """ + raw = _compose(jmap) + if raw is None: + return + parsed = email.message_from_bytes(raw) + for jmap_key, header in (("to", "To"), ("cc", "Cc")): + value = parsed[header] + if not value: + continue + supplied = sum(1 for m in jmap[jmap_key] if (m.get("email") or "").strip()) + assert _separator_commas(str(value)) + 1 <= supplied + + @settings(**FUZZ_SETTINGS) + @given(jmap=jmap_email_dict) + def test_no_header_is_smuggled(self, jmap): + """Composed output must not carry a header we never set.""" + raw = _compose(jmap) + if raw is None: + return + parsed = email.message_from_bytes(raw) + allowed = { + "from", + "to", + "cc", + "bcc", + "reply-to", + "sender", + "subject", + "date", + "message-id", + "in-reply-to", + "references", + "mime-version", + "content-type", + "content-transfer-encoding", + "content-disposition", + "content-id", + } + for name in parsed.keys(): + assert str(name).lower() in allowed, f"smuggled header {name!r}" + + @settings(**FUZZ_SETTINGS) + @given(jmap=jmap_email_dict) + def test_body_cannot_forge_a_mime_part(self, jmap): + """A body carrying ``--boundary`` must not split the message. + + This is what an unpredictable boundary buys: the part count is a + function of what we built, never of what the body said. + """ + raw = _compose(jmap) + if raw is None: + return + parsed = email.message_from_bytes(raw) + if parsed.is_multipart(): + assert len(parsed.get_payload()) <= 2 + + @settings(**FUZZ_SETTINGS) + @given(jmap=jmap_email_dict) + def test_no_address_appears_that_we_never_supplied(self, jmap): + """Recovery is best-effort on exotic input; invention is not.""" + raw = _compose(jmap) + if raw is None: + return + reparsed = parse_email(raw) + assert reparsed is not None + supplied = { + (m.get("email") or "").strip() + for m in jmap["to"] + if is_valid_addr_spec((m.get("email") or "").strip()) + } + # A display name is not an address, however much it looks like + # one — this is the property that caught the composer + # RFC 2047-encoding addr-specs, which made a lenient re-parse + # fall back to the decoded display name as the recipient. + recovered = {a["email"] for a in (reparsed.get("to") or [])} + assert recovered <= supplied or not supplied + + @settings(**FUZZ_SETTINGS) + @given(jmap=jmap_email_dict) + def test_boundary_is_fresh_every_compose(self, jmap): + """Two composes agree on the headers and differ on the boundary — + if they matched, it would be predictable.""" + a, b = _compose(jmap), _compose(jmap) + if a is None or b is None: + assert a is None and b is None + return + pa, pb = email.message_from_bytes(a), email.message_from_bytes(b) + assert pa["Subject"] == pb["Subject"] + assert pa["To"] == pb["To"] + if pa.is_multipart(): + assert pa.get_boundary() != pb.get_boundary() diff --git a/src/jmap-email/tests/test_wire_roundtrip_fuzz.py b/src/jmap-email/tests/test_wire_roundtrip_fuzz.py new file mode 100644 index 00000000..023cd960 --- /dev/null +++ b/src/jmap-email/tests/test_wire_roundtrip_fuzz.py @@ -0,0 +1,216 @@ +""" +Fuzzing the wire round trip: raw bytes → parse → compose → parse. + +``test_roundtrip_fuzz`` starts from synthetic JMAP dicts. This one starts +from *bytes an attacker wrote*, which is the path a mail system actually +walks: inbound MIME is parsed, the result is edited into a reply or a +forward, and the composer turns it back into bytes. Anything the parser +mis-reports becomes something the composer emits under our own +signature. + +The properties are about fidelity and non-amplification across that +loop. Fidelity, because a forward that changes the recipient list is a +bug; non-amplification, because a message that grows an address, a +header or a part each time it is forwarded is a weapon. + +Run with: pytest -m fuzz tests/test_wire_roundtrip_fuzz.py +Or: make fuzz-jmap-email +""" + +import email +import os +from collections import Counter + +import pytest +from hypothesis import HealthCheck, given, settings +from hypothesis import strategies as st + +from jmap_email import ComposeError, compose_email, parse_email + +FUZZ_SETTINGS = { + # Override for a deeper soak: FUZZ_EXAMPLES=100000 make fuzz-jmap-email + "max_examples": int(os.environ.get("FUZZ_EXAMPLES", "10000")), + "deadline": None, + "suppress_health_check": [HealthCheck.too_slow, HealthCheck.data_too_large], + # Phases are Hypothesis's defaults on purpose. ``shrink`` and + # ``explain`` cost nothing on a green run — they only engage once a + # failure exists, which is exactly when you want a minimal example + # rather than the raw generated blob. ``reuse`` replays a stored + # failure until it is fixed, which is what makes an intermittent + # find reproducible; it needs ``.hypothesis`` to survive the + # container, so compose mounts it. +} + +# Header lines assembled from fragments that mean something structural. +# The point is to build messages a real MTA might hand us, not uniform +# noise: the interesting inputs are almost-valid. +_header_line = st.one_of( + st.sampled_from( + [ + b"From: a@b.co", + b"From: alice@good.co", + b"From: mallory@evil.co", + b'From: "=?utf-8?B?ZXZpbEB4LmNv?=" ', + b"From: =?utf-8?B?ZXZpbEB4LmNv?= ", + b"To: b@c.co", + b"To: b@c.co, d@e.co", + b"To: ", + b'To: "a b"@c.co', + b"Cc: e@f.co", + b"Subject: hi", + b"Subject: =?utf-8?B?w6l0w6k=?=", + b"Subject: =?utf-8?q?a=0d=0aBcc:_e@x.co?=", + b"Date: Mon, 8 Jun 2026 12:00:00 +0200", + b"Date: not-a-date", + b"Message-ID: ", + b"Message-ID: ", + b"In-Reply-To: ", + b"References: ", + b"MIME-Version: 1.0", + b"Content-Transfer-Encoding: base64", + b"Content-Transfer-Encoding: bas64", + b"Reply-To: r@s.co", + b"X-Custom: v", + ] + ), +) + +_body_block = st.sampled_from( + [ + b"plain body\n", + b"--x\nContent-Type: text/plain\n\npart one\n--x--\n", + b"preamble text\n--x\nContent-Type: text/plain\n\npart\n--x--\nepilogue\n", + b"--x\nContent-Type: application/pdf\n" + b'Content-Disposition: attachment; filename="r.pdf"\n\nQUFB\n--x--\n', + b"--x\nContent-Type: application/pdf\n" + b"Content-Disposition: attachment; " + b"filename=\"safe.txt\"; filename*=UTF-8''evil.exe\n\nQUFB\n--x--\n", + b"--x\nContent-Type: message/rfc822\n\nFrom: i@n.co\nSubject: in\n\nx\n--x--\n", + b"U01VR0dMRUQ=\n", + b"U01VR0!!dMRUQ=\n", + b"", + ] +) + +raw_message = st.builds( + lambda headers, ct, body: b"\n".join(headers) + b"\n" + ct + b"\n\n" + body, + st.lists(_header_line, min_size=1, max_size=7), + st.sampled_from( + [ + b"Content-Type: text/plain", + b'Content-Type: multipart/mixed; boundary="x"', + b'Content-Type: multipart/mixed; boundary="x"; boundary="y"', + b"Content-Type: multipart/mixed", + b"Content-Type: garbage", + ] + ), + _body_block, +) + + +def _addrs(parsed, key): + """Address multiset for *key*. + + A ``Counter``, not a ``set``: amplification is the property under + test, and a set collapses a recipient that gained a duplicate into + one entry — exactly the bug these assertions exist to catch. + """ + return Counter(a["email"] for a in (parsed.get(key) or [])) + + +@pytest.mark.fuzz +class TestWireRoundTrip: + """parse → compose → parse must not invent, and must not amplify.""" + + @settings(**FUZZ_SETTINGS) + @given(raw=raw_message) + def test_parse_never_raises_on_wire_bytes(self, raw): + """The documented contract: a single ``is None`` check, no except.""" + parsed = parse_email(raw, extensions=True) + assert parsed is None or isinstance(parsed, dict) + + @settings(**FUZZ_SETTINGS) + @given(raw=raw_message) + def test_recompose_invents_no_address(self, raw): + """Forwarding must not add a recipient. + + The composer signs what it emits with our identity, so an address + that appears only after the round trip is one we vouched for and + the sender never wrote. + """ + first = parse_email(raw) + if first is None: + return + try: + rebuilt = compose_email(first) + except ComposeError: + return + second = parse_email(rebuilt) + assert second is not None + for key in ("from", "to", "cc"): + assert _addrs(second, key) <= _addrs(first, key) + + @settings(**FUZZ_SETTINGS) + @given(raw=raw_message) + def test_round_trip_reaches_a_fixed_point(self, raw): + """A second lap must change nothing a third lap would change. + + Mail is forwarded repeatedly. A loop that keeps mutating the + message — dropping a recipient each time, or re-encoding a + subject — corrupts a thread by attrition rather than all at once. + """ + first = parse_email(raw) + if first is None: + return + try: + once = compose_email(first) + except ComposeError: + return + mid = parse_email(once) + assert mid is not None + try: + twice = compose_email(mid) + except ComposeError: + pytest.fail("compose accepted its own output once but not twice") + end = parse_email(twice) + assert end is not None + for key in ("from", "to", "cc"): + assert _addrs(end, key) == _addrs(mid, key) + assert end["subject"] == mid["subject"] + + @settings(**FUZZ_SETTINGS) + @given(raw=raw_message) + def test_headers_do_not_multiply(self, raw): + """No header may gain a copy by being round-tripped. + + A duplicated identity header is the spoof the parser flags on the + way in; emitting one on the way out would manufacture it. + """ + first = parse_email(raw) + if first is None: + return + try: + rebuilt = compose_email(first) + except ComposeError: + return + emitted = email.message_from_bytes(rebuilt) + names = [str(k).lower() for k in emitted.keys()] + for singleton in ("from", "to", "cc", "subject", "date", "message-id"): + assert names.count(singleton) <= 1, f"{singleton} emitted twice" + + @settings(**FUZZ_SETTINGS) + @given(raw=raw_message) + def test_attachments_do_not_multiply(self, raw): + """Forwarding must not duplicate a payload.""" + first = parse_email(raw) + if first is None: + return + try: + rebuilt = compose_email(first) + except ComposeError: + return + second = parse_email(rebuilt) + assert second is not None + assert len(second.get("attachments") or []) <= len( + first.get("attachments") or [] + )