/logs and /metrics: a bearer token with a non-ASCII character answers 500 instead of 401 #372

Closed
opened 2026-10-01 10:46:13 +00:00 by tiagoagueda · 0 comments
Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the two token checks and sending them malformed Authorization headers.

Checked again on 2026-10-01 against main at 0a5cad587: still present.

Both endpoints compare the presented token with hmac.compare_digest(presented.strip(), expected), on two str values. Django decodes request headers as Latin-1, so any header byte above 0x7F becomes a non-ASCII character, and compare_digest refuses to compare non-ASCII strings: it raises TypeError. The request fails with a 500 and a logged traceback, where it should get the same 401 as any other wrong token.

  • Where: core/views_logs.py:49-63 (_authorised, called from collect at :82) and core/views_metrics.py:25-34 (_authorised, called from scrape at :41).
  • What happens: with either endpoint enabled and a token configured, a request whose bearer token holds a byte such as 0xE9 raises TypeError: comparing strings with non-ASCII characters is not supported inside _authorised. Checked against the snapshot: 500, with the traceback in the log.
  • Who can trigger it: anyone who can reach the instance, with no token, once an operator has enabled /logs or /metrics. Each such request writes an ERROR record with a traceback, and because the check comes before throttle.endpoint, nothing limits how many.
  • Why it matters: a crash on reachable anonymous input, and an easy way to fill the error log that an operator watches (and that /logs itself serves) with noise.
  • Fix: compare bytes: hmac.compare_digest(presented.strip().encode(), expected.encode()), which accepts any input and still runs in constant time. One helper for both views would stop the two copies drifting apart again.

Worth deciding alongside, not part of the defect: wrong tokens are never counted. tests/security/test_rate_limits.py:385-395 (test_a_wrong_token_is_still_refused_before_anything_is_counted) makes that a deliberate choice, on the grounds that there is no work to protect behind a bad token. Operator tokens are usually long, but nothing checks their strength, so a separate bucket for failed attempts per address (answering 429 once spent, and leaving the good-token budget alone) would bound guessing at /logs, which serves the whole application log.

Done when

  • tests/security/test_logs_endpoint.py and tests/security/test_metrics.py each send Authorization: Bearer followed by a Latin-1 byte above 0x7F and assert a 401. Both fail today with a 500 and pass after the fix.
Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the two token checks and sending them malformed `Authorization` headers. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. Both endpoints compare the presented token with `hmac.compare_digest(presented.strip(), expected)`, on two `str` values. Django decodes request headers as Latin-1, so any header byte above 0x7F becomes a non-ASCII character, and `compare_digest` refuses to compare non-ASCII strings: it raises `TypeError`. The request fails with a 500 and a logged traceback, where it should get the same 401 as any other wrong token. - **Where:** `core/views_logs.py:49-63` (`_authorised`, called from `collect` at `:82`) and `core/views_metrics.py:25-34` (`_authorised`, called from `scrape` at `:41`). - **What happens:** with either endpoint enabled and a token configured, a request whose bearer token holds a byte such as `0xE9` raises `TypeError: comparing strings with non-ASCII characters is not supported` inside `_authorised`. Checked against the snapshot: 500, with the traceback in the log. - **Who can trigger it:** anyone who can reach the instance, with no token, once an operator has enabled `/logs` or `/metrics`. Each such request writes an ERROR record with a traceback, and because the check comes before `throttle.endpoint`, nothing limits how many. - **Why it matters:** a crash on reachable anonymous input, and an easy way to fill the error log that an operator watches (and that `/logs` itself serves) with noise. - **Fix:** compare bytes: `hmac.compare_digest(presented.strip().encode(), expected.encode())`, which accepts any input and still runs in constant time. One helper for both views would stop the two copies drifting apart again. Worth deciding alongside, not part of the defect: wrong tokens are never counted. `tests/security/test_rate_limits.py:385-395` (`test_a_wrong_token_is_still_refused_before_anything_is_counted`) makes that a deliberate choice, on the grounds that there is no work to protect behind a bad token. Operator tokens are usually long, but nothing checks their strength, so a separate bucket for failed attempts per address (answering 429 once spent, and leaving the good-token budget alone) would bound guessing at `/logs`, which serves the whole application log. ## Done when - `tests/security/test_logs_endpoint.py` and `tests/security/test_metrics.py` each send `Authorization: Bearer` followed by a Latin-1 byte above 0x7F and assert a 401. Both fail today with a 500 and pass after the fix.
tiagoagueda added this to the 0.6.0 milestone 2026-10-01 10:46:13 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
Postulo/postulo#372
No description provided.