BodySizeLimitInterceptor bypassed by missing or malformed Content-Length

uwe.admin commented

2026-04-25 13:24:09 +02:00

Owner

Location: include/oatpp-authkit/interceptor/BodySizeLimitInterceptor.hpp:26-38

The interceptor only inspects Content-Length and silently passes requests with a missing or malformed header (the bare catch (...) then "let it through"). Trivial bypass via chunked transfer encoding or HTTP/2. A library named "body size limit" should either reject missing/unparsable Content-Length or measure streamed body size after parsing.

From audit #1.

**Location:** `include/oatpp-authkit/interceptor/BodySizeLimitInterceptor.hpp:26-38` The interceptor only inspects `Content-Length` and silently passes requests with a missing or malformed header (the bare `catch (...)` then "let it through"). Trivial bypass via chunked transfer encoding or HTTP/2. A library named "body size limit" should either reject missing/unparsable `Content-Length` or measure streamed body size after parsing. From audit #1.

uwe.admin referenced this issue

2026-04-25 13:24:26 +02:00

Code Audit Report — 2026-04-23 #1

uwe.admin referenced this issue

2026-04-25 13:24:26 +02:00

Code Audit Report — 2026-04-23 #1

uwe.admin commented

2026-04-25 13:35:19 +02:00

Author

Owner

Agent Evaluation

Feasibility: Small. The interceptor is 30 lines; the change is in intercept().

Impact: Medium. The current behaviour is a misleading false-sense-of-security: a header named "body size limit" suggests it caps body size, but a request with no Content-Length (or a malformed one) sails through, with a code-comment that even acknowledges this. Concrete bypasses: chunked transfer encoding, HTTP/2 (no Content-Length mandate), misbehaving clients, deliberate omission. Once bypassed, the body falls through to oatpp's much higher default ceiling. Tightening this closes a real hole for any consumer that thinks the interceptor is enforcing a hard cap.

Effort: Small.

Recommendation: Accept.

Implementation plan

Treat missing Content-Length the same as oversized: return 411 Length Required (or 413 per consumer preference) by default. Add a bool requireContentLength ctor flag (default true to fail-closed).
Treat malformed Content-Length (current bare catch (...)) the same — reject as 400 Bad Request. No more silent pass-through.
Update doc comment: explicitly call out that this interceptor relies on a trustworthy Content-Length; consumers who need to support chunked transfer encoding should set requireContentLength = false and rely on a stream-layer guard.
Test (in the harness from #2): assert valid CL ok / oversized 413 / missing CL 411 / malformed CL 400 / opt-out flag lets missing-CL through.

Decision needed

Check one (edit this comment):

Option A — Fail-closed by default — missing or malformed Content-Length becomes 411/400; consumers add requireContentLength=false if they need lax behaviour. Safer default; potentially breaks consumers that quietly relied on the bypass.
Option B — Fail-open by default, opt-in to strict — keep current behaviour but add requireContentLength=true flag. Backwards-compatible; consumers who never opt in remain vulnerable. Defeats most of the point.

Default recommendation: A. The library's purpose is hardened defaults; consumers who need the bypass can opt out explicitly.

## Agent Evaluation **Feasibility:** Small. The interceptor is 30 lines; the change is in `intercept()`. **Impact:** Medium. The current behaviour is a misleading false-sense-of-security: a header named "body size limit" suggests it caps body size, but a request with no `Content-Length` (or a malformed one) sails through, with a code-comment that even acknowledges this. Concrete bypasses: chunked transfer encoding, HTTP/2 (no `Content-Length` mandate), misbehaving clients, deliberate omission. Once bypassed, the body falls through to oatpp's much higher default ceiling. Tightening this closes a real hole for any consumer that thinks the interceptor is enforcing a hard cap. **Effort:** Small. **Recommendation:** Accept. ### Implementation plan 1. Treat **missing `Content-Length`** the same as oversized: return `411 Length Required` (or `413` per consumer preference) by default. Add a `bool requireContentLength` ctor flag (default `true` to fail-closed). 2. Treat **malformed `Content-Length`** (current bare `catch (...)`) the same — reject as `400 Bad Request`. No more silent pass-through. 3. Update doc comment: explicitly call out that this interceptor relies on a trustworthy `Content-Length`; consumers who need to support chunked transfer encoding should set `requireContentLength = false` and rely on a stream-layer guard. 4. Test (in the harness from #2): assert valid CL ok / oversized 413 / missing CL 411 / malformed CL 400 / opt-out flag lets missing-CL through. ### Decision needed Check one (edit this comment): - [ ] **Option A — Fail-closed by default** — missing or malformed `Content-Length` becomes 411/400; consumers add `requireContentLength=false` if they need lax behaviour. Safer default; potentially breaks consumers that quietly relied on the bypass. - [ ] **Option B — Fail-open by default, opt-in to strict** — keep current behaviour but add `requireContentLength=true` flag. Backwards-compatible; consumers who never opt in remain vulnerable. Defeats most of the point. Default recommendation: **A**. The library's purpose is hardened defaults; consumers who need the bypass can opt out explicitly.

uwe.admin added the

evaluated

label 2026-04-25 13:35:19 +02:00

uwe.admin commented

2026-04-25 13:35:19 +02:00

Author

Owner

Evaluated #4 — Small, recommend accept; A/B decision on fail-closed vs fail-open default.

uwe.admin referenced this issue

2026-04-25 13:41:37 +02:00

Replace ad-hoc JSON parsing/serialising with oatpp ObjectMapper #6