vCard writer: a lone carriage return in a name or note, or a line break in the email, adds properties to the card #27

New issue

Closed

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

tiagoagueda commented

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

Owner

Found on 2026-09-29 in a read-only review of postulo-dav at 97abc9779, comparing the plugin's vCard writer with the core's iCalendar writer and rendering cards with control characters in them.

The plugin's escape handles \r\n and \n but leaves a lone \r in the content line, and EMAIL and URL are written with no treatment at all. A vCard is a list of lines, and the plugin's own unfold, like other readers, takes a lone \r as a line break, so whatever follows it is read as a property of its own. This is content-line injection, the weakness postulo/postulo#218 (closed) fixed in the core's writer (applications/ical.py:63-71: \r folded first, then every control character removed). The plugin keeps its own copy of the rule, and the copy never received the fix.

Where: src/postulo_dav/text.py:48-55 (escape), src/postulo_dav/text.py:165 (EMAIL;TYPE=work:{card.email}) and :169 (URL:{card.url}) in card_to_text, src/postulo_dav/text.py:22-32 (unfold, which splits on \r).
What happens: a card built with the note first, a carriage return, then TITLE:Injected was rendered and read back with text_to_card: the note was first and the title Injected. The same with a carriage return in the name gave a card with an ORG nobody typed, and with a plain \n in the email a TITLE. The pushed card carries the extra property into the address book. When that card is next pulled, apply_card writes the result back: the notes lose everything after the break and the role, company or email take the injected value.
Who can trigger it: anything that puts text into a contact or its company. The API passes name, role, notes and email through as sent (api/schemas.py:131-139; email is a plain str), so a token holder or a tool acting through one can. A captured company name keeps interior control characters (JobPostingData strips only the ends, plugins/base.py:42) and is written as ORG. Text pasted from a source that uses bare \r does it by accident. The target is the account holder's own address book and, from there, every device that syncs it.
Why it matters: text from outside can add or replace properties on cards in the person's address book, and a later pull turns that into lost notes and changed fields here.
Fix: make escape do what the core's does: text.replace("\r\n", "\n").replace("\r", "\n") before escaping, and drop every remaining character below U+0020 and U+007F. Strip the same control characters from EMAIL and URL (they are not text values, so no backslash escaping). ical.escape is deliberately not on the plugin surface (#4), so the copy stays here; the test below is what keeps it from drifting again.

Done when

tests/test_dav.py has a test that renders a card with \r, \r\n, \n and other control characters in the name, company, role and note, and a line break in the email and the URL, and asserts that text_to_card returns no property that was not set and that the rendered text has no \r outside the \r\n line endings.

Found on 2026-09-29 in a read-only review of postulo-dav at 97abc9779, comparing the plugin's vCard writer with the core's iCalendar writer and rendering cards with control characters in them. The plugin's `escape` handles `\r\n` and `\n` but leaves a lone `\r` in the content line, and `EMAIL` and `URL` are written with no treatment at all. A vCard is a list of lines, and the plugin's own `unfold`, like other readers, takes a lone `\r` as a line break, so whatever follows it is read as a property of its own. This is content-line injection, the weakness postulo/postulo#218 (closed) fixed in the core's writer (`applications/ical.py:63-71`: `\r` folded first, then every control character removed). The plugin keeps its own copy of the rule, and the copy never received the fix. - **Where:** `src/postulo_dav/text.py:48-55` (`escape`), `src/postulo_dav/text.py:165` (`EMAIL;TYPE=work:{card.email}`) and `:169` (`URL:{card.url}`) in `card_to_text`, `src/postulo_dav/text.py:22-32` (`unfold`, which splits on `\r`). - **What happens:** a card built with the note `first`, a carriage return, then `TITLE:Injected` was rendered and read back with `text_to_card`: the note was `first` and the title `Injected`. The same with a carriage return in the name gave a card with an `ORG` nobody typed, and with a plain `\n` in the email a `TITLE`. The pushed card carries the extra property into the address book. When that card is next pulled, `apply_card` writes the result back: the notes lose everything after the break and the role, company or email take the injected value. - **Who can trigger it:** anything that puts text into a contact or its company. The API passes `name`, `role`, `notes` and `email` through as sent (`api/schemas.py:131-139`; `email` is a plain `str`), so a token holder or a tool acting through one can. A captured company name keeps interior control characters (`JobPostingData` strips only the ends, `plugins/base.py:42`) and is written as `ORG`. Text pasted from a source that uses bare `\r` does it by accident. The target is the account holder's own address book and, from there, every device that syncs it. - **Why it matters:** text from outside can add or replace properties on cards in the person's address book, and a later pull turns that into lost notes and changed fields here. - **Fix:** make `escape` do what the core's does: `text.replace("\r\n", "\n").replace("\r", "\n")` before escaping, and drop every remaining character below U+0020 and U+007F. Strip the same control characters from `EMAIL` and `URL` (they are not text values, so no backslash escaping). `ical.escape` is deliberately not on the plugin surface (#4), so the copy stays here; the test below is what keeps it from drifting again. ## Done when - `tests/test_dav.py` has a test that renders a card with `\r`, `\r\n`, `\n` and other control characters in the name, company, role and note, and a line break in the email and the URL, and asserts that `text_to_card` returns no property that was not set and that the rendered text has no `\r` outside the `\r\n` line endings.

tiagoagueda added this to the 0.5.0 milestone

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

tiagoagueda added the

bug

security

labels

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

tiagoagueda referenced this issue from Postulo/postulo

2026-10-03 01:58:35 +00:00

Write down the contribution rules: look for a library before writing one, and the formats data leaves in #653

tiagoagueda referenced this issue

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