http: walk chunk framing to find the end of a chunked response

[yosuke-wolfssl]

Problem

read_body() treated a Transfer-Encoding: chunked response as complete as
soon as the bytes 0\r\n\r\n appeared anywhere in the buffer, unanchored to
chunk framing. Two failure modes, both on valid responses:

• Payload containing those bytes stops the read early, so decode_chunked()
  finds no terminal chunk and returns WOLFCERT_ERR_PROTOCOL. Needs the false
  terminator in an earlier segment than the rest of the body — the normal case
  for a multi-segment PKCS#7 body.
• A terminal chunk followed by trailer fields never contains that 5-byte
  run, so the reader waits for a close that never comes. On keep-alive, with
  io_timeout_ms = -1, an unbounded hang.

Reached by every blocking one-shot and session EST/SCEP request. Severity High.
Closes f-8010.

Fix (src/http.c)

• read_chunk_size() parses one size line, reporting ok / need-more /
  malformed. Skips a ; extension, rejects a digitless line, caps the value at
  0xFFFFFFFF so it cannot wrap a later bounds check.
• chunked_body_complete() walks size lines positionally, verifies the CRLF
  closing each payload, and completes only at a zero-length chunk whose trailer
  section is closed by a blank line.
• check_trailers() rejects a trailer line with no field name, and a
  section that no blank line closes.
• decode_chunked() shares the size parser, so the walk and the decode pass
  cannot drift. Also drops an unvalidated strtoul() where a size line of zz
  parsed as 0 and returned WOLFCERT_OK with a truncated body.

Mirrors the server-side fix in src/est/est_server.c (f-6876).

Tests (tests/unit/test_http.c)

Twenty-three cases via chunk_body_case() (one response, optionally split
across two segments) and trailer_split_case() (two requests on one keep-alive
connection). Segmentation matters: a single-send() server does not reproduce
the bug.

Area	Cases
False terminators	payload holding 0\r\n\r\n; 10 size line whose payload opens with CRLF
Trailers	whole; repeated; split at a field line and at the closing CRLF
Malformed trailers	no colon; empty field name; leading colon
Size lines	extension, lowercase, uppercase, zero-padded, non-hex, empty, split across reads, ending a segment
Framing errors	stray byte where a chunk's CRLF belongs; truncated close
Limits	empty body; over max_response_bytes in one chunk and across several

TIMEOUT 30 on the http target and alarm(25) in main() bound the run,
since a framing bug surfaces as a hang.

Verification

Clean build, no warnings. 27/27 ctest. ASan + UBSan clean. No leaks. Each new
test mutation-checked: reverting a guard fails that test and only that test;
re-injecting the original scan fails three.

Not in this PR

The three helpers are duplicated in src/est/est_server.c rather than shared,
and the copies now differ on the size cap, the delimiter check and trailer
validation — better fixed by extracting a shared helper than by mirroring each
change. Bodyless-response framing (f-8020) is separate.

[Copilot]

🔵 Needs a closer look

Two moderate findings remain in tests/unit/test_http.c regarding missing NUL terminators.

Pull request overview

Fixes HTTP chunked-response framing by parsing chunk boundaries and trailers positionally, preventing premature completion and hangs.

Changes:

• Validates chunk sizes, payload delimiters, trailers, and size limits.
• Adds comprehensive malformed-input and split-read tests.
• Adds a 30-second HTTP test timeout.

File summaries

File	Summary
tests/unit/test_http.c	Adds chunked-response coverage. Moderate findings (1 vote each) at lines 800 and 819: constructed buffers need NUL termination before strlen use.
tests/CMakeLists.txt	Adds an HTTP test execution timeout.
src/http.c	Implements positional chunk framing, trailer-aware completion, and shared validated decoding.

Review details

Suppressed comments (2)

tests/unit/test_http.c:800

• body is passed to srv_chunk_body_thread, which calls strlen(cs_ctx->body). The final memcpy overwrites the NUL emitted by snprintf, and this array is never terminated afterward, so this test reads past body and may send arbitrary stack bytes. Add a terminator before passing it to the helper.

    memcpy(body + n, "\r\n0\r\n\r\n", 8);

tests/unit/test_http.c:819

• As in the single-chunk case, body is passed to a helper that uses strlen, but this final memcpy does not append a NUL terminator. The test therefore has undefined behavior and can include bytes beyond the constructed response; terminate the buffer after the six-byte trailer.

    memcpy(body + n, "0\r\n\r\n", 6);

• Files reviewed: 3/3 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #26

Scan targets checked: wolfcert-bugs

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #26

Scan targets checked: wolfcert-bugs

Fenrir result: Approved ✅

No new issues found in the changed files.

Advisory only — this automated result does not count as a GitHub approval.

Fenrir's latest completed scan found no issues; clearing the prior automated change request.