SEC2-034: The request id is judged by the rewritten client address, so a trusted proxy's `X-Request-ID` is dropped #976

New issue

Closed

opened 2026-10-17 10:38:57 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-10-17 10:38:57 +00:00

Owner

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

Severity: Informational · Category: Configuration · Reference: CWE-807 (Reliance on Untrusted Inputs in a Security Decision); OWASP A09:2021 (Security Logging and Monitoring Failures)

Location:

src/postulo/core/middleware.py:244-249 (RequestIDMiddleware.__call__; the check is line 248)
src/postulo/core/proxy.py:146-154 (TrustedProxyMiddleware rewrites REMOTE_ADDR and keeps the peer under POSTULO_PROXY_ADDR)
src/postulo/core/proxy.py:101 (request_origin() already reads the peer the right way)

Issue

TrustedProxyMiddleware runs first and, when the peer is a trusted proxy that sent X-Forwarded-For, replaces REMOTE_ADDR with the client's address and records the proxy under POSTULO_PROXY_ADDR. RequestIDMiddleware runs second and asks whether the rewritten address is a trusted proxy:

given = request.headers.get(logs.REQUEST_ID_HEADER, "")
if given and not proxy.is_trusted(request.META.get("REMOTE_ADDR", "")):
    given = ""

In the deployment the feature exists for (a proxy in POSTULO_TRUSTED_PROXIES that sets both X-Forwarded-For and X-Request-ID), REMOTE_ADDR is by then the visitor's public address, is_trusted is false, and the proxy's id is thrown away. Postulo's log, gunicorn's and the proxy's can no longer be laid side by side, which was #233's purpose. tests/test_hardening_798.py tests only a bare loopback peer and a bare stranger; no test sends X-Forwarded-For with the id.

Impact

Loss of log correlation in the common proxied deployment: a reviewer cannot pair a proxy's access line with Postulo's lines for the same request. No exposure.

Fix

Judge by the peer that delivered the request, which the first middleware preserved, as proxy.request_origin() does (reuse it rather than repeating the expression):

peer = request.META.get("POSTULO_PROXY_ADDR") or request.META.get("REMOTE_ADDR", "")
if given and not proxy.is_trusted(peer):
    given = ""

Done when: tests/test_hardening_798.py gains two cases: REMOTE_ADDR=172.18.0.5 with POSTULO_TRUSTED_PROXIES=["172.18.0.0/16"], HTTP_X_FORWARDED_FOR=203.0.113.9 and an id, which is kept; and REMOTE_ADDR=203.0.113.9 (untrusted) with the same forwarded header and an id, which is replaced.

#798 (closed) - its suggested remediation names POSTULO_PROXY_ADDR; the fix that landed reads REMOTE_ADDR after it has been rewritten, so this is an incomplete fix of a closed issue. #233 (closed) - the feature. #393 (closed) - the API's instance field uses the id.

Found on 2026-10-16 in a read-only security review of `postulo` at `56fc9d531`, checked by a second, independent pass. **Severity:** Informational · **Category:** Configuration · **Reference:** CWE-807 (Reliance on Untrusted Inputs in a Security Decision); OWASP A09:2021 (Security Logging and Monitoring Failures) **Location:** - `src/postulo/core/middleware.py:244-249` (`RequestIDMiddleware.__call__`; the check is line 248) - `src/postulo/core/proxy.py:146-154` (`TrustedProxyMiddleware` rewrites `REMOTE_ADDR` and keeps the peer under `POSTULO_PROXY_ADDR`) - `src/postulo/core/proxy.py:101` (`request_origin()` already reads the peer the right way) ## Issue `TrustedProxyMiddleware` runs first and, when the peer is a trusted proxy that sent `X-Forwarded-For`, replaces `REMOTE_ADDR` with the client's address and records the proxy under `POSTULO_PROXY_ADDR`. `RequestIDMiddleware` runs second and asks whether the *rewritten* address is a trusted proxy: ```python given = request.headers.get(logs.REQUEST_ID_HEADER, "") if given and not proxy.is_trusted(request.META.get("REMOTE_ADDR", "")): given = "" ``` In the deployment the feature exists for (a proxy in `POSTULO_TRUSTED_PROXIES` that sets both `X-Forwarded-For` and `X-Request-ID`), `REMOTE_ADDR` is by then the visitor's public address, `is_trusted` is false, and the proxy's id is thrown away. Postulo's log, gunicorn's and the proxy's can no longer be laid side by side, which was #233's purpose. `tests/test_hardening_798.py` tests only a bare loopback peer and a bare stranger; no test sends `X-Forwarded-For` with the id. ## Impact Loss of log correlation in the common proxied deployment: a reviewer cannot pair a proxy's access line with Postulo's lines for the same request. No exposure. ## Fix Judge by the peer that delivered the request, which the first middleware preserved, as `proxy.request_origin()` does (reuse it rather than repeating the expression): ```python peer = request.META.get("POSTULO_PROXY_ADDR") or request.META.get("REMOTE_ADDR", "") if given and not proxy.is_trusted(peer): given = "" ``` Done when: `tests/test_hardening_798.py` gains two cases: `REMOTE_ADDR=172.18.0.5` with `POSTULO_TRUSTED_PROXIES=["172.18.0.0/16"]`, `HTTP_X_FORWARDED_FOR=203.0.113.9` and an id, which is kept; and `REMOTE_ADDR=203.0.113.9` (untrusted) with the same forwarded header and an id, which is replaced. ## Related #798 (closed) - its suggested remediation names `POSTULO_PROXY_ADDR`; the fix that landed reads `REMOTE_ADDR` after it has been rewritten, so this is an incomplete fix of a closed issue. #233 (closed) - the feature. #393 (closed) - the API's `instance` field uses the id.

tiagoagueda added the

enhancement

security

labels

2026-10-17 10:38:57 +00:00

tiagoagueda added this to the 1.0.0 milestone

2026-10-17 10:38:58 +00:00

tiagoagueda referenced this issue

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-18 22:20:57 +00:00