Sync: one message that cannot be parsed fails the whole pass, and every pass after it #8

New issue

Closed

opened 2026-10-01 10:47:55 +00:00 by tiagoagueda · 1 comment

tiagoagueda commented

2026-10-01 10:47:55 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo-imap at cc09c6381, running parse and ImapSync.sync against malformed messages.

Mailbox.fetch parses every message of a pass in one loop with nothing around each one, and parse has reachable inputs that raise an exception nobody catches. The exception leaves sync() before anything is flagged or moved, so no message of that pass produces a suggestion, and the next pass fetches the same message and fails the same way. The folder stays unread until the message is older than the look-back (thirty days by default) or the person finds it and removes it by hand.

Where:
- src/postulo_imap/mailbox.py:204-207, Mailbox.fetch: mails.append(parse(int(uid), raw)) with no try.
- src/postulo_imap/mailbox.py:138-142, parse: email.utils.parsedate_to_datetime(message["Date"]) under except (TypeError, ValueError).
- src/postulo_imap/mailbox.py:121-125, _payload: raw.decode(charset, errors="replace") under except LookupError.
- src/postulo_imap/sync.py:175 (the fetch) and src/postulo_imap/sync.py:214-217 (the flag or the move, never reached); sync() has only finally: close(client).
What happens: three inputs, the first two run against the snapshot on Python 3.14, which is what the image ships.
1. A Date header that contains a raw 8-bit byte. The compat32 parser hands such a header back as an email.header.Header object, not a str, and parsedate_to_datetime calls .split() on it: AttributeError: 'Header' object has no attribute 'split'. AttributeError is not in the tuple that is caught.
2. A part whose declared charset names a codec that does not accept errors="replace". charset=idna raises UnicodeError: Unsupported error handling: replace; charset=undefined raises UnicodeError: undefined encoding. Only LookupError is caught.
3. A NUL character in the subject, the sender's name or the body. It survives _decoded and plain_text (parse returned 'x\x00y' and 'bo\x00dy') and reaches suggest() as summary, body and context. PostgreSQL refuses NUL in text and jsonb values, so on a PostgreSQL instance the insert raises inside the loop at src/postulo_imap/sync.py:188. This third case was not run (SQLite accepts NUL); an RFC 2047 encoded word or a base64 body is enough to carry one through any server.
With a fake client holding one ordinary acknowledgement and one message of the first kind, ImapSync().sync() raised the AttributeError with nothing flagged and no suggestion filed. The core's sync_connection (plugins/syncing.py:79-85) records AttributeError: 'Header' object has no attribute 'split' as the connection's last error and moves on; it cannot skip a message. Because the search is NOT KEYWORD $Postulo SINCE <date>, the same message comes back on every pass.
Who can trigger it: anyone whose mail reaches the folder the connection reads: a recruiting system that writes a malformed header, or unsolicited mail the person's filter files there. No account on the instance is needed.
Why it matters: the plugin stops working for that person, with an error that names a Python attribute and not a message. Mail that arrives while the bad message is still inside the look-back is not read; mail that ages out together with it is never read at all, so its acknowledgements, rejections and invitations never become suggestions.
Fix:
- In Mailbox.fetch, parse each message inside its own try/except Exception: log the uid and the exception, count it (a skipped on the report and a note saying how many could not be read), and keep the uid in the list that is flagged or moved so it is not fetched again.
- In sync(), give each message's suggest() call the same isolation, so one row the database refuses does not stop the rest.
- In parse, pass str(message.get("Date")) or catch Exception around the date; catch (LookupError, UnicodeError) in _payload; remove NUL characters from every string before it leaves parse. Parsing with policy=email.policy.default removes the Header-object case altogether, at the price of re-checking the other header reads.

Done when

tests/test_imap.py has a test that parse returns a Mail (with date None or the parsed date) for a Date header carrying an 8-bit byte, for charset=idna and for charset=undefined, and that no string of the returned Mail contains NUL when the subject and the body did. All three fail today.
tests/test_imap.py has a test that runs a pass over one good acknowledgement and one message parse cannot read: the suggestion for the good one is filed, both are flagged $Postulo, and the report says one message could not be read. It fails today with AttributeError.

Found on 2026-09-29 in a read-only review of postulo-imap at cc09c6381, running `parse` and `ImapSync.sync` against malformed messages. `Mailbox.fetch` parses every message of a pass in one loop with nothing around each one, and `parse` has reachable inputs that raise an exception nobody catches. The exception leaves `sync()` before anything is flagged or moved, so no message of that pass produces a suggestion, and the next pass fetches the same message and fails the same way. The folder stays unread until the message is older than the look-back (thirty days by default) or the person finds it and removes it by hand. - **Where:** - `src/postulo_imap/mailbox.py:204-207`, `Mailbox.fetch`: `mails.append(parse(int(uid), raw))` with no `try`. - `src/postulo_imap/mailbox.py:138-142`, `parse`: `email.utils.parsedate_to_datetime(message["Date"])` under `except (TypeError, ValueError)`. - `src/postulo_imap/mailbox.py:121-125`, `_payload`: `raw.decode(charset, errors="replace")` under `except LookupError`. - `src/postulo_imap/sync.py:175` (the fetch) and `src/postulo_imap/sync.py:214-217` (the flag or the move, never reached); `sync()` has only `finally: close(client)`. - **What happens:** three inputs, the first two run against the snapshot on Python 3.14, which is what the image ships. 1. A `Date` header that contains a raw 8-bit byte. The compat32 parser hands such a header back as an `email.header.Header` object, not a `str`, and `parsedate_to_datetime` calls `.split()` on it: `AttributeError: 'Header' object has no attribute 'split'`. `AttributeError` is not in the tuple that is caught. 2. A part whose declared charset names a codec that does not accept `errors="replace"`. `charset=idna` raises `UnicodeError: Unsupported error handling: replace`; `charset=undefined` raises `UnicodeError: undefined encoding`. Only `LookupError` is caught. 3. A NUL character in the subject, the sender's name or the body. It survives `_decoded` and `plain_text` (`parse` returned `'x\x00y'` and `'bo\x00dy'`) and reaches `suggest()` as `summary`, `body` and `context`. PostgreSQL refuses NUL in `text` and `jsonb` values, so on a PostgreSQL instance the insert raises inside the loop at `src/postulo_imap/sync.py:188`. This third case was not run (SQLite accepts NUL); an RFC 2047 encoded word or a base64 body is enough to carry one through any server. With a fake client holding one ordinary acknowledgement and one message of the first kind, `ImapSync().sync()` raised the `AttributeError` with nothing flagged and no suggestion filed. The core's `sync_connection` (`plugins/syncing.py:79-85`) records `AttributeError: 'Header' object has no attribute 'split'` as the connection's last error and moves on; it cannot skip a message. Because the search is `NOT KEYWORD $Postulo SINCE <date>`, the same message comes back on every pass. - **Who can trigger it:** anyone whose mail reaches the folder the connection reads: a recruiting system that writes a malformed header, or unsolicited mail the person's filter files there. No account on the instance is needed. - **Why it matters:** the plugin stops working for that person, with an error that names a Python attribute and not a message. Mail that arrives while the bad message is still inside the look-back is not read; mail that ages out together with it is never read at all, so its acknowledgements, rejections and invitations never become suggestions. - **Fix:** - In `Mailbox.fetch`, parse each message inside its own `try`/`except Exception`: log the uid and the exception, count it (a `skipped` on the report and a note saying how many could not be read), and keep the uid in the list that is flagged or moved so it is not fetched again. - In `sync()`, give each message's `suggest()` call the same isolation, so one row the database refuses does not stop the rest. - In `parse`, pass `str(message.get("Date"))` or catch `Exception` around the date; catch `(LookupError, UnicodeError)` in `_payload`; remove NUL characters from every string before it leaves `parse`. Parsing with `policy=email.policy.default` removes the `Header`-object case altogether, at the price of re-checking the other header reads. ## Done when - `tests/test_imap.py` has a test that `parse` returns a `Mail` (with `date` `None` or the parsed date) for a `Date` header carrying an 8-bit byte, for `charset=idna` and for `charset=undefined`, and that no string of the returned `Mail` contains NUL when the subject and the body did. All three fail today. - `tests/test_imap.py` has a test that runs a pass over one good acknowledgement and one message `parse` cannot read: the suggestion for the good one is filed, both are flagged `$Postulo`, and the report says one message could not be read. It fails today with `AttributeError`.

tiagoagueda added this to the 0.5.0 milestone

2026-10-01 10:47:55 +00:00

tiagoagueda added the

bug

label

2026-10-01 10:47:55 +00:00

tiagoagueda commented

2026-10-17 10:40:32 +00:00

Author

Owner

From the 2026-10-16 security review: a fourth input that fails the whole pass, and it raises in match(), not in parse() or suggest().

A message whose References header carries more identifiers than the database accepts as bound parameters makes match() (matching.py:90-98, one IN clause over every identifier) raise OperationalError inside the per-message loop of sync() (sync.py:182-212). Nothing around it catches it; box.mark(uids) / box.move(...) run only after the loop (sync.py:214-217), so nothing is flagged and the next pass fails on the same message until it leaves the look-back window. The count itself is #15's to bound; this comment is about the try.

The fix proposed here wraps parse in Mailbox.fetch and suggest() in sync(). Please make the per-message try in sync() wrap the whole body of the loop, classify, match() and suggest() together, so that any one message that raises becomes report.skipped += 1 plus a note and is still flagged.

Done when a test makes match() raise for one message of a pass and asserts the other messages are filed and flagged and the report names one skipped.

From the 2026-10-16 security review: a fourth input that fails the whole pass, and it raises in `match()`, not in `parse()` or `suggest()`. A message whose `References` header carries more identifiers than the database accepts as bound parameters makes `match()` (`matching.py:90-98`, one `IN` clause over every identifier) raise `OperationalError` inside the per-message loop of `sync()` (`sync.py:182-212`). Nothing around it catches it; `box.mark(uids)` / `box.move(...)` run only after the loop (`sync.py:214-217`), so nothing is flagged and the next pass fails on the same message until it leaves the look-back window. The count itself is #15's to bound; this comment is about the `try`. The fix proposed here wraps `parse` in `Mailbox.fetch` and `suggest()` in `sync()`. Please make the per-message `try` in `sync()` wrap the whole body of the loop, `classify`, `match()` and `suggest()` together, so that any one message that raises becomes `report.skipped += 1` plus a note and is still flagged. Done when a test makes `match()` raise for one message of a pass and asserts the other messages are filed and flagged and the report names one skipped.

tiagoagueda referenced this issue from Postulo/postulo

2026-10-17 10:41:23 +00:00

SEC2 index: security review of 2026-10-16 (86 issues across the core, plugins and extensions) #992

tiagoagueda referenced this issue from a commit

2026-10-19 07:00:13 +00:00

Fix the review findings: reading, classifying, matching, the form and what a refusal does

tiagoagueda referenced this issue from a pull request that will close it,

2026-10-19 07:00:17 +00:00

Fix the open bug and security issues (#8-#22, #24) #25

tiagoagueda closed this issue

2026-10-19 09:55:26 +00:00