Connection form: STARTTLS with the port left empty dials 993, and the look-back takes any integer #13

New issue

Closed

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

tiagoagueda commented

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

Owner

Found on 2026-09-29 in a read-only review of postulo-imap at cc09c6381, reading how the form's optional numbers reach the connection and the search, and running both on the snapshot.

Two optional integer fields are used without being checked. The port falls back to 993 whatever security was chosen, so the choice labelled "STARTTLS (port 143)" does not use 143 unless the person types it. The look-back accepts negative and very large numbers: the first makes a connection that looks healthy and never reads anything, the second fails with a raw OverflowError.

1. The port does not follow the security

Where: src/postulo_imap/mailbox.py:266-267, connect(): port = int(config.get("port") or 993) is computed before starttls is known. src/postulo_imap/sync.py:47-57: the port is required=False, default=993, and the choices are labelled "TLS (port 993)" and "STARTTLS (port 143)".
What happens: the form opens with 993 in the port field. A person on a STARTTLS-only server picks STARTTLS (port 143) and either leaves the 993 that was put there for them or clears the field, trusting the label. Both store a configuration that calls IMAPClient(address, port=993, ssl=False, …) (confirmed with a stand-in client for "", None and 993): a plain connection to the implicit-TLS port, which waits for a greeting until TIMEOUT (30 seconds) and then reports "Could not reach the server. Check the address, the port and the security." The message does name the port, but the form has just told the person that STARTTLS means 143. Test runs inside the request, so the page hangs for that half minute.
Fix: give the port no default and choose it from the security when it is empty: int(config.get("port") or (143 if starttls else 993)), or pass None and let IMAPClient choose, which it already does (port = ssl and 993 or 143). In validate(), refuse a port outside 1–65535.

2. The look-back is not bounded

Where: src/postulo_imap/sync.py:109-116 declares days as an optional integer; sync.py:149 and sync.py:172 pass int(config.get("days") or mailbox.DEFAULT_SINCE_DAYS); src/postulo_imap/mailbox.py:185 computes (dt.datetime.now(dt.UTC) - dt.timedelta(days=since_days)).date(). validate() (sync.py:123-129) looks only at folder and move_to, and the core builds a plain forms.IntegerField with no minimum or maximum (plugins/forms.py:131-132 in postulo), so -30 and 99999999999999 both pass the form.
What happens:
- With -30, the search is SINCE a date thirty days in the future (run on the snapshot: SINCE 2026-10-31 on 2026-10-01). Test answers "Signed in. 0 messages … are waiting to be read." and every pass reports nothing to do, for ever, with no error anywhere.
- With 800000 the subtraction raises OverflowError: date value out of range; with 1000000000000, dt.timedelta raises OverflowError: Python int too large to convert to C int. Neither is a MailboxError, so test() (sync.py:152) does not catch it; the core's catch-all turns it into "Test failed: OverflowError: date value out of range" (plugins/views.py:402-404) and into the connection's last error on every scheduled pass (plugins/syncing.py:83-85).
- 0 is falsy and silently becomes thirty.
Fix: in validate(), require 1 <= days <= a sane maximum (3650, say) and put the problem against days.

Done when

tests/test_imap.py has a test in which a configuration with security = starttls and an empty port reaches the fake client with port == 143; it fails today with 993.
tests/test_imap.py has a test in which ImapSync().validate() names days for -30, 0 and 10**12, and port for 0 and 70000; all return {} today.

Found on 2026-09-29 in a read-only review of postulo-imap at cc09c6381, reading how the form's optional numbers reach the connection and the search, and running both on the snapshot. Two optional integer fields are used without being checked. The port falls back to 993 whatever security was chosen, so the choice labelled "STARTTLS (port 143)" does not use 143 unless the person types it. The look-back accepts negative and very large numbers: the first makes a connection that looks healthy and never reads anything, the second fails with a raw `OverflowError`. ## 1. The port does not follow the security - **Where:** `src/postulo_imap/mailbox.py:266-267`, `connect()`: `port = int(config.get("port") or 993)` is computed before `starttls` is known. `src/postulo_imap/sync.py:47-57`: the port is `required=False, default=993`, and the choices are labelled "TLS (port 993)" and "STARTTLS (port 143)". - **What happens:** the form opens with 993 in the port field. A person on a STARTTLS-only server picks *STARTTLS (port 143)* and either leaves the 993 that was put there for them or clears the field, trusting the label. Both store a configuration that calls `IMAPClient(address, port=993, ssl=False, …)` (confirmed with a stand-in client for `""`, `None` and `993`): a plain connection to the implicit-TLS port, which waits for a greeting until `TIMEOUT` (30 seconds) and then reports "Could not reach the server. Check the address, the port and the security." The message does name the port, but the form has just told the person that STARTTLS means 143. *Test* runs inside the request, so the page hangs for that half minute. - **Fix:** give the port no default and choose it from the security when it is empty: `int(config.get("port") or (143 if starttls else 993))`, or pass `None` and let IMAPClient choose, which it already does (`port = ssl and 993 or 143`). In `validate()`, refuse a port outside 1–65535. ## 2. The look-back is not bounded - **Where:** `src/postulo_imap/sync.py:109-116` declares `days` as an optional integer; `sync.py:149` and `sync.py:172` pass `int(config.get("days") or mailbox.DEFAULT_SINCE_DAYS)`; `src/postulo_imap/mailbox.py:185` computes `(dt.datetime.now(dt.UTC) - dt.timedelta(days=since_days)).date()`. `validate()` (`sync.py:123-129`) looks only at `folder` and `move_to`, and the core builds a plain `forms.IntegerField` with no minimum or maximum (`plugins/forms.py:131-132` in postulo), so `-30` and `99999999999999` both pass the form. - **What happens:** - With `-30`, the search is `SINCE` a date thirty days in the future (run on the snapshot: `SINCE 2026-10-31` on 2026-10-01). *Test* answers "Signed in. 0 messages … are waiting to be read." and every pass reports nothing to do, for ever, with no error anywhere. - With `800000` the subtraction raises `OverflowError: date value out of range`; with `1000000000000`, `dt.timedelta` raises `OverflowError: Python int too large to convert to C int`. Neither is a `MailboxError`, so `test()` (`sync.py:152`) does not catch it; the core's catch-all turns it into "Test failed: OverflowError: date value out of range" (`plugins/views.py:402-404`) and into the connection's last error on every scheduled pass (`plugins/syncing.py:83-85`). - `0` is falsy and silently becomes thirty. - **Fix:** in `validate()`, require `1 <= days <= ` a sane maximum (3650, say) and put the problem against `days`. ## Done when - `tests/test_imap.py` has a test in which a configuration with `security` = `starttls` and an empty port reaches the fake client with `port == 143`; it fails today with 993. - `tests/test_imap.py` has a test in which `ImapSync().validate()` names `days` for `-30`, `0` and `10**12`, and `port` for `0` and `70000`; all return `{}` today.

tiagoagueda added this to the 0.6.0 milestone

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

tiagoagueda added the

bug

label

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

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

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

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