Fetching a page or a logo: a capture address can forge a log line, NAT64 addresses pass the public-only check, and the size caps apply after the whole body is read #321

New issue

Closed

opened 2026-09-29 19:45:59 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-09-29 19:45:59 +00:00

Owner

Found on 2026-09-29 while re-checking #249's dismissed CodeQL findings against f097f1591, by a reviewer trying to break them rather than confirm them. None of these is in #249's table, and CodeQL flags none of them: it does not treat a django-ninja payload as a source, and it does not model httpx's buffering.

1. A capture address can write a forged line to the console log (CWE-117, low)

Where: plugins/registry.py:383 ("Capture source %r failed on %s", url), :408 ("Could not list the places on %s", url) and plugins/builtin/__init__.py:446 ("Filling gaps with %s failed on %s") log the address with %s.
How it gets there: POST /api/v1/captures and …/captures/preview pass PageIn.url (api/api.py:185) into remembered.read_page and registry.parse_page. The only check is core/addresses.py:44-63, which looks at the scheme and returns the value unchanged, newline included. The web capture form is not affected: it uses forms.URLField, which drops newlines.
Shown: calling parse_page with https://x.example/a\n2026-09-29 12:00:00,000 WARNING postulo.core.server_views: FORGED and a page that makes a source raise wrote that second line to stdout as a separate, well-formed WARNING line.
Who can do it: any account holder with an API token that has the captures scope.
What it reaches: stdout under the default POSTULO_LOG_FORMAT=simple. The file log served at /logs is JSON, one json.dumps per record, so it cannot be forged.
Fix: refuse control characters in page_address (an address with a newline in it is not an address), and log addresses with %r.

2. The public-only check lets NAT64 and IPv4-compatible IPv6 addresses through (SSRF, depends on the host)

Where: plugins/fetching.py:128 and core/destinations.py:73 rely on ipaddress's is_global.
What it misses: is_global is true for 64:ff9b::/96 (NAT64), ::/96 (IPv4-compatible) and fec0::/10 (the retired site-local range).
Why it matters: on a host with NAT64, 64:ff9b::a9fe:a9fe reaches 169.254.169.254, the cloud metadata address. On an IPv4-only host these addresses go nowhere, so the risk depends on the deployment.
Fix: unwrap every IPv4-embedding form (::ffff:0:0/96, 64:ff9b::/96, 64:ff9b:1::/48, ::/96, 6to4 2002::/16, Teredo 2001::/32) and judge the IPv4 address inside it, and refuse fec0::/10. Test each form with a private IPv4 address inside it.

3. The size caps run only after the whole body is in memory (availability)

Where: the 2 MB cap in fetching._read_capped (:183-196, called after client.get at :226) and the 5 MB cap in logos.download (:121, :139).
What happens: httpx reads the whole response when stream=False (_client.py:921-922). Its 8 s and 10 s timeouts apply to each read, not to the whole download.
The attack: a hostile public server, which is all a capture or a logo address needs to name, can make a worker buffer a response of any size, sent slowly enough never to trip a timeout.
Fix: stream the body (client.stream(...), iter_bytes), stop at the cap, and put a limit on the whole download's time as well as each read's. Test with a transport that yields more than the cap and one that yields slowly.

4. An imported username skips the username rules (hardening)

Where: core/importer.py:356-363 sets user.username from an archive with only .strip().casefold(): no pattern and no blacklist.
Why it is low: it is reachable only through the operator-run import_data command. The pattern in accounts/validators.py:18 also ends in $, which accepts a trailing newline ("bob\n" passes). Every web path strips the value, and User.save() strips its ends.
Fix: run the username validator and the blacklist in the importer, and anchor the pattern with \Z (or use fullmatch).

Done when

Each point has its test: a forged-line attempt that is refused at the address; each embedded-IPv4 form carrying a private address that is refused; an oversized and a slow body that stop at the cap; an archive with an invalid username that is refused with a message.
tests/security/test_outbound.py covers the new address forms.

Found on 2026-09-29 while re-checking #249's dismissed CodeQL findings against `f097f1591`, by a reviewer trying to break them rather than confirm them. None of these is in #249's table, and CodeQL flags none of them: it does not treat a django-ninja payload as a source, and it does not model `httpx`'s buffering. ## 1. A capture address can write a forged line to the console log (CWE-117, low) - **Where:** `plugins/registry.py:383` (`"Capture source %r failed on %s", url`), `:408` (`"Could not list the places on %s", url`) and `plugins/builtin/__init__.py:446` (`"Filling gaps with %s failed on %s"`) log the address with `%s`. - **How it gets there:** `POST /api/v1/captures` and `…/captures/preview` pass `PageIn.url` (`api/api.py:185`) into `remembered.read_page` and `registry.parse_page`. The only check is `core/addresses.py:44-63`, which looks at the scheme and returns the value unchanged, newline included. The web capture form is not affected: it uses `forms.URLField`, which drops newlines. - **Shown:** calling `parse_page` with `https://x.example/a\n2026-09-29 12:00:00,000 WARNING postulo.core.server_views: FORGED` and a page that makes a source raise wrote that second line to stdout as a separate, well-formed WARNING line. - **Who can do it:** any account holder with an API token that has the `captures` scope. - **What it reaches:** stdout under the default `POSTULO_LOG_FORMAT=simple`. The file log served at `/logs` is JSON, one `json.dumps` per record, so it cannot be forged. - **Fix:** refuse control characters in `page_address` (an address with a newline in it is not an address), and log addresses with `%r`. ## 2. The public-only check lets NAT64 and IPv4-compatible IPv6 addresses through (SSRF, depends on the host) - **Where:** `plugins/fetching.py:128` and `core/destinations.py:73` rely on `ipaddress`'s `is_global`. - **What it misses:** `is_global` is true for `64:ff9b::/96` (NAT64), `::/96` (IPv4-compatible) and `fec0::/10` (the retired site-local range). - **Why it matters:** on a host with NAT64, `64:ff9b::a9fe:a9fe` reaches `169.254.169.254`, the cloud metadata address. On an IPv4-only host these addresses go nowhere, so the risk depends on the deployment. - **Fix:** unwrap every IPv4-embedding form (`::ffff:0:0/96`, `64:ff9b::/96`, `64:ff9b:1::/48`, `::/96`, 6to4 `2002::/16`, Teredo `2001::/32`) and judge the IPv4 address inside it, and refuse `fec0::/10`. Test each form with a private IPv4 address inside it. ## 3. The size caps run only after the whole body is in memory (availability) - **Where:** the 2 MB cap in `fetching._read_capped` (`:183-196`, called after `client.get` at `:226`) and the 5 MB cap in `logos.download` (`:121`, `:139`). - **What happens:** `httpx` reads the whole response when `stream=False` (`_client.py:921-922`). Its 8 s and 10 s timeouts apply to each read, not to the whole download. - **The attack:** a hostile public server, which is all a capture or a logo address needs to name, can make a worker buffer a response of any size, sent slowly enough never to trip a timeout. - **Fix:** stream the body (`client.stream(...)`, `iter_bytes`), stop at the cap, and put a limit on the whole download's time as well as each read's. Test with a transport that yields more than the cap and one that yields slowly. ## 4. An imported username skips the username rules (hardening) - **Where:** `core/importer.py:356-363` sets `user.username` from an archive with only `.strip().casefold()`: no pattern and no blacklist. - **Why it is low:** it is reachable only through the operator-run `import_data` command. The pattern in `accounts/validators.py:18` also ends in `$`, which accepts a trailing newline (`"bob\n"` passes). Every web path strips the value, and `User.save()` strips its ends. - **Fix:** run the username validator and the blacklist in the importer, and anchor the pattern with `\Z` (or use `fullmatch`). ## Done when - Each point has its test: a forged-line attempt that is refused at the address; each embedded-IPv4 form carrying a private address that is refused; an oversized and a slow body that stop at the cap; an archive with an invalid username that is refused with a message. - `tests/security/test_outbound.py` covers the new address forms.

tiagoagueda added this to the 0.5.0 milestone

2026-09-29 19:45:59 +00:00

tiagoagueda added the

bug

security

labels

2026-09-29 19:45:59 +00:00

tiagoagueda referenced this issue

2026-09-29 19:46:00 +00:00

CodeQL: a small cluster of real quality findings, and the guards it cannot see #249

tiagoagueda referenced this issue

2026-10-01 10:46:04 +00:00