Skip to content

Bound the cache-index (.ndx) parser to its buffer - #507

Merged
xroche merged 2 commits into
masterfrom
fix-cache-ndx-oob
Jul 8, 2026
Merged

Bound the cache-index (.ndx) parser to its buffer#507
xroche merged 2 commits into
masterfrom
fix-cache-ndx-oob

Conversation

@xroche

@xroche xroche commented Jul 8, 2026

Copy link
Copy Markdown
Owner

A corrupt or truncated hts-cache/*.ndx walked the cache loader's length-prefixed scan past the end of the readfile() buffer: an out-of-bounds read reachable on --update/--continue, or from a cache truncated by a crash or a full disk.

cache_brstr advanced its cursor by the on-disk length prefix (clamped only to 32768) without checking those bytes were actually present, so a short file whose declared length overstates it sent the cursor past the terminating NUL; the next binput()/strchr() in the loader then read out of bounds. The fix bounds both the copy and the advance to the bytes actually there (strnlen up to the NUL), and adds cache_binput(), a binput() that refuses to start a read at or past end-of-buffer, for the two loader scan loops. The listing loop in htscoremain.c also picks up the a < end guard the main htscache.c loop already had.

-#test=cacheindex gates it: the deterministic bound check fails on the old advance, and the ASan CI jobs exercise the scan itself. Found with the cache-index fuzz harness in the follow-up PR.

Also adds dist/ to .gitignore: httrack-gh-release stages signed tarballs there, and the missing rule let them slip into an earlier revision of this PR.

xroche and others added 2 commits July 8, 2026 20:36
cache_brstr trusted the on-disk length prefix for its cursor advance
(off += i, i up to 32768), so a corrupt or truncated hts-cache/*.ndx
whose declared length overstates the file walked the length-prefixed scan
past the end of the readfile() buffer; the next binput()/strchr() in the
loader then read out of bounds. A cache truncated by a crash or a full
disk, or one handed to --update/--continue, reaches it.

Bound both the copy and the advance in cache_brstr to the bytes actually
present (strnlen up to the terminating NUL), and add cache_binput(), a
binput() that refuses to start a read at or past end-of-buffer, for the
two loader scan loops in htscache.c and htscoremain.c; the latter also
gains the a < end guard the former already had. Regression: -#test=cacheindex
plus 01_engine-cacheindex.test (the deterministic bound check fails on the
old advance; the ASan CI jobs also exercise the scan).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
httrack-gh-release stages its signed tarballs in dist/ inside the
checkout; only root-level /httrack-*.tar.gz was ignored, so a broad
git add swept the 3.49.10/3.49.11 artifacts into this branch's first
commit (since rewritten out). Ignore the directory.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Xavier Roche <roche@httrack.com>
@xroche
xroche force-pushed the fix-cache-ndx-oob branch from e8a883d to ce6ca52 Compare July 8, 2026 18:37
@xroche
xroche merged commit 65c1016 into master Jul 8, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant