diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 2b5d819c..44eaa59b 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -90,3 +90,8 @@ **Vulnerability:** The `_safe_upload_filename` function used `filename.replace`, `PurePosixPath`, and `re.sub` on unbounded client input, making it vulnerable to ReDoS or CPU/memory exhaustion (DoS) when fed extremely long strings. **Learning:** Even fast standard library functions like `PurePosixPath` and string replacements can cause significant lag when chained on strings in the megabytes. String processing operations should always bound their inputs first if the input is untrusted and can be arbitrarily large. **Prevention:** Cap the length of client-provided filename strings early by slicing them (e.g. `filename = filename[-512:]`) before doing more complex string parsing or regex replacements, especially when only the basename suffix is relevant. + +## 2026-07-28 - Prevent Resource Exhaustion (DoS) in Authentication +**Vulnerability:** The `/parse` API accepted unbound length strings in the `Authorization` header, which were then passed directly to the expensive `hmac.compare_digest` function. If an attacker sent an extremely long header, this could result in Resource Exhaustion and a Denial of Service (DoS) attack. +**Learning:** Even constant-time comparison functions like `hmac.compare_digest` can cause performance degredation if the strings they are comparing are arbitrarily large. We must ensure user-controlled input string lengths are strictly bounded before passing them to expensive functions. +**Prevention:** Enforce strict length limits (e.g., max 512 characters) on incoming strings in `require_authorization` before passing them to expensive cryptographic comparison functions. diff --git a/src/newsdom_api/main.py b/src/newsdom_api/main.py index 4efdad56..b8422606 100644 --- a/src/newsdom_api/main.py +++ b/src/newsdom_api/main.py @@ -133,7 +133,7 @@ def require_authorization( return expected = f"Bearer {token}" provided = authorization or "" - if not hmac.compare_digest(provided, expected): + if len(provided) > 512 or not hmac.compare_digest(provided, expected): raise HTTPException( status_code=401, detail=UNAUTHORIZED_DETAIL, diff --git a/tests/test_auth.py b/tests/test_auth.py index 2dc94fa2..365d2956 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -89,3 +89,30 @@ def test_get_api_token_strips_surrounding_whitespace(monkeypatch): def test_config_module_exposes_env_var_name(): assert config.API_TOKEN_ENV_VAR == "NEWSDOM_API_TOKEN" + + +def test_parse_rejects_overlong_bearer_token_without_calling_compare_digest( + monkeypatch, stub_parser +): + monkeypatch.setenv(API_TOKEN_ENV_VAR, "s3cret-token") + + called = False + import hmac + + original_compare = hmac.compare_digest + + def mock_compare_digest(a, b): + nonlocal called + called = True + return original_compare(a, b) + + monkeypatch.setattr("hmac.compare_digest", mock_compare_digest) + + client = TestClient(app) + response = client.post( + "/parse", + files=_PDF_FILES, + headers={"Authorization": "Bearer " + "a" * 600}, + ) + assert response.status_code == 401 + assert not called, "hmac.compare_digest should not be called for overlong strings"