SEC2-059: One malformed date or an over-long remote identifier fails the whole run, on every run #34

New issue

Closed

opened 2026-10-17 10:39:52 +00:00 by tiagoagueda · 1 comment

tiagoagueda commented

2026-10-17 10:39:52 +00:00

Owner

Found on 2026-10-16 in a read-only security review of postulo-dav at 1d49d6e, checked by a second, independent pass.

Severity: Low · Category: Input Validation · Reference: CWE-20 (Improper Input Validation); CWE-755 (Improper Handling of Exceptional Conditions); OWASP A04:2021 (Insecure Design)

Location:

src/postulo_dav/text.py:270 (_STAMP: (\d{2}) for month, day, hour, minute and second)
src/postulo_dav/text.py:277-283 (parse_stamp: dt.datetime(y, mo, d, h, mi, s, tzinfo=dt.UTC) with no try)
src/postulo_dav/text.py:286-303 (parse_moment: the constructors at :293 and :303; only the ZoneInfo lookup is guarded)
src/postulo_dav/sync.py:195-199 (apply_event: reschedule_interview(...) with no check that the end follows the start)
src/postulo_dav/sync.py:473,534,607-608 (every pull parses with nothing around it)
src/postulo_dav/sync.py:543-551 (SyncLink.bind(..., remote_href=href, uid=card.uid, ...) uncut); core src/postulo/plugins/models.py:385-386 (remote_href 500, uid 200)

Issue

_STAMP matches any two digits for the month, day, hour, minute and second, and the values go
straight into datetime(...), which raises ValueError for a month of 13 or an hour of 25.
Nothing catches it: text_to_card and text_to_events propagate it out of
_sync_contacts/_sync_interviews, so a single REV, DTSTART, DTEND or LAST-MODIFIED of
that shape ends the run before the ETag is stored, and the next run reads the same resource and
fails the same way. The core records the exception text on the connection
(plugins/syncing.py:126-128), and the interviews half never runs when the card is the problem
(sync.py:405-416).

Two more inputs fail alike. An adopted card's href over 500 characters or UID over 200 is written
to SyncLink uncut; on PostgreSQL that raises at bind after the contact was already created,
the pattern #20 describes for the name and company (SQLite enforces no length, so this part
affects the PostgreSQL profile only). And apply_event reschedules with whatever two moments
parsed, so an event whose end precedes its start is stored as such; Interview.ends_at has no
constraint against starts_at.

Impact

Whoever can write to the collection the connection reads (the person on a phone with a lenient
client, another user of a shared server) can stop the connection for good until the record is
found and fixed by hand; the person sees only a ValueError: month must be in 1..12 line. On
PostgreSQL the adoption case also leaves one more unlinked contact behind each run, as in #20.

Fix

One change in four parts, so that one bad resource costs that resource and not the run:

parse_stamp and parse_moment return None on ValueError from the constructor (one line
each; #24's fix for unknown zones then extends the same guard).
apply_event reschedules only when ends_at > starts_at, and otherwise notes the event.
Before bind, cut remote_href to 500 and uid to 200, or skip the card with a note.
One per-resource try around each pull and adoption that turns an exception into
report.skipped += 1 and a note. #20's per-card atomic() block is the same step seen from
the database side; the two should land together.

Done when tests feed a card with REV:20261301T000000Z, an event with
DTSTART:20260931T250000Z, an event whose DTEND precedes its DTSTART, and an adopted card
with a 300-character UID, and assert that the run completes, the interview is not moved, and the
report names what was skipped.

#20 (open): the same failure class for Contact.name and Company.name and the unlinked contact
left behind; dates and the SyncLink columns are not in it. #24 (open): parse_moment and
zones; it asks for None on an unknown zone, not for the constructor's own ValueError. #2
(closed) item "a failure in one half not sink the other" is noted in #20 as not done.

Found on 2026-10-16 in a read-only security review of `postulo-dav` at `1d49d6e`, checked by a second, independent pass. **Severity:** Low · **Category:** Input Validation · **Reference:** CWE-20 (Improper Input Validation); CWE-755 (Improper Handling of Exceptional Conditions); OWASP A04:2021 (Insecure Design) **Location:** - `src/postulo_dav/text.py:270` (`_STAMP`: `(\d{2})` for month, day, hour, minute and second) - `src/postulo_dav/text.py:277-283` (`parse_stamp`: `dt.datetime(y, mo, d, h, mi, s, tzinfo=dt.UTC)` with no `try`) - `src/postulo_dav/text.py:286-303` (`parse_moment`: the constructors at `:293` and `:303`; only the `ZoneInfo` lookup is guarded) - `src/postulo_dav/sync.py:195-199` (`apply_event`: `reschedule_interview(...)` with no check that the end follows the start) - `src/postulo_dav/sync.py:473,534,607-608` (every pull parses with nothing around it) - `src/postulo_dav/sync.py:543-551` (`SyncLink.bind(..., remote_href=href, uid=card.uid, ...)` uncut); core `src/postulo/plugins/models.py:385-386` (`remote_href` 500, `uid` 200) ## Issue `_STAMP` matches any two digits for the month, day, hour, minute and second, and the values go straight into `datetime(...)`, which raises `ValueError` for a month of 13 or an hour of 25. Nothing catches it: `text_to_card` and `text_to_events` propagate it out of `_sync_contacts`/`_sync_interviews`, so a single `REV`, `DTSTART`, `DTEND` or `LAST-MODIFIED` of that shape ends the run before the ETag is stored, and the next run reads the same resource and fails the same way. The core records the exception text on the connection (`plugins/syncing.py:126-128`), and the interviews half never runs when the card is the problem (`sync.py:405-416`). Two more inputs fail alike. An adopted card's href over 500 characters or UID over 200 is written to `SyncLink` uncut; on PostgreSQL that raises at `bind` after the contact was already created, the pattern #20 describes for the name and company (SQLite enforces no length, so this part affects the PostgreSQL profile only). And `apply_event` reschedules with whatever two moments parsed, so an event whose end precedes its start is stored as such; `Interview.ends_at` has no constraint against `starts_at`. ## Impact Whoever can write to the collection the connection reads (the person on a phone with a lenient client, another user of a shared server) can stop the connection for good until the record is found and fixed by hand; the person sees only a `ValueError: month must be in 1..12` line. On PostgreSQL the adoption case also leaves one more unlinked contact behind each run, as in #20. ## Fix One change in four parts, so that one bad resource costs that resource and not the run: 1. `parse_stamp` and `parse_moment` return `None` on `ValueError` from the constructor (one line each; #24's fix for unknown zones then extends the same guard). 2. `apply_event` reschedules only when `ends_at > starts_at`, and otherwise notes the event. 3. Before `bind`, cut `remote_href` to 500 and `uid` to 200, or skip the card with a note. 4. One per-resource `try` around each pull and adoption that turns an exception into `report.skipped += 1` and a note. #20's per-card `atomic()` block is the same step seen from the database side; the two should land together. Done when tests feed a card with `REV:20261301T000000Z`, an event with `DTSTART:20260931T250000Z`, an event whose `DTEND` precedes its `DTSTART`, and an adopted card with a 300-character UID, and assert that the run completes, the interview is not moved, and the report names what was skipped. ## Related #20 (open): the same failure class for `Contact.name` and `Company.name` and the unlinked contact left behind; dates and the `SyncLink` columns are not in it. #24 (open): `parse_moment` and zones; it asks for `None` on an unknown zone, not for the constructor's own `ValueError`. #2 (closed) item "a failure in one half not sink the other" is noted in #20 as not done.

tiagoagueda added the

bug

security

labels

2026-10-17 10:39:53 +00:00

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:13:10 +00:00

Review fixes: pairing by UID, bounded reads, per-resource tolerance, notes per reader, and a calendar that carries no attendee

tiagoagueda referenced this issue