Compare commits

..

2 Commits

Author SHA1 Message Date
Xavier Roche
349c07f4ca Bound the in-memory 206-resume allocation to a 32-bit size
CodeQL flagged the memory-branch resume buffer as an uncontrolled allocation
size: malloct(resume + 1 + totalsize) takes an attacker-controlled
Content-Length. #534 guarded the int64 add against overflow but left the
allocation itself unbounded, and the (size_t) cast still truncated on a 32-bit
build. Cap the buffer at INT32_MAX -- a resource held whole in RAM is far
smaller -- and route an over-large or overflowing size to the existing
drop-and-refetch path. Test 48_local-crange-memresume already exercises that
path (its INT64_MAX Content-Length now trips the tighter bound).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
2026-07-11 20:17:18 +02:00
Xavier Roche
797acecfd2 Harden Content-Range integer handling against hostile and oversize values (#534)
* Harden Content-Range integer handling against hostile and oversize values

Two integer-safety defects in the Content-Range / size path, both reachable
from a hostile or buggy server and abort-worthy under the project's UBSan/
OSS-Fuzz CI (a sanitizer abort is a real bug, not flakiness).

C2 (signed-overflow UB). The signed int64 crange fields are parsed straight
from the header with no validation, so a crafted Content-Range drives the
crange +/- 1 range checks past the int64 extremes. Clamp any negative field to
the all-zero "no range" sentinel at parse time (this covers the parse-side
crange-1 fallback and the 200->206 fake-range check), and rewrite the 206
resume check crange_end + 1 == crange as crange_end == crange - 1, which is
guarded by the preceding crange > 0 so an INT64_MAX end cannot overflow.

C4 (off_t truncated to int). The sizehack read the on-disk size into a 32-bit
int, truncating it for files >= 2 GiB, so a same-size match could force a 304
that records the wrong size (cache/disk metadata desync, never a truncated
file). Widen to off_t like every sibling call site.

Tests: -#test=crange drives the parser (UBSan aborts pre-fix on the INT64_MIN
fallback); 47_local-crange-overflow drives the 206-resume path with an
INT64_MAX Content-Range (UBSan aborts pre-fix at the crange+1 check).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>

* Also guard the in-memory 206-resume buffer size against a hostile Content-Length

An adversarial review of the crange hardening surfaced a third overflow in the
same 206-resume path: the in-memory branch sizes its buffer as
`resume + 1 + totalsize`, and `totalsize` (Content-Length) is attacker-
controlled and unclamped. A resume answered with a 206 that lies `text/html`
(so the resume buffers in memory) and a matching INT64_MAX Content-Length drives
the add to INT64_MAX + 1 — a signed-overflow UBSan abort, reachable from a single
hostile response. Reject a Content-Length that would overflow the size, drop the
partial, and refetch from scratch (the retry restarts clean).

Test 48_local-crange-memresume drives it end to end (UBSan aborts pre-fix at
`alloc_mem += totalsize`); 01_engine-crange gains a `-5-10/20` case exercising
the clamp's independent per-field negativity.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>

---------

Signed-off-by: Xavier Roche <roche@httrack.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-11 20:04:47 +02:00

View File

@@ -3908,10 +3908,11 @@ void back_wait(struct_back * sback, httrackp * opt, cache_back * cache,
if (fp) {
LLint alloc_mem = resume + 1;
// Reject a hostile Content-Length that would
// overflow the buffer size: drop the partial and
// refetch.
if (back[i].r.totalsize > INT64_MAX - alloc_mem) {
// Bound the in-memory buffer to a 32-bit size (real
// in-RAM resources are far smaller); a hostile
// Content-Length that would overflow the add or the
// (size_t) cast is dropped and refetched instead.
if (back[i].r.totalsize > INT32_MAX - alloc_mem) {
url_savename_refname_remove(opt, back[i].url_adr,
back[i].url_fil);
UNLINK(back[i].url_sav);