Three hand-written copies of the same clipping contract#741
Merged
Conversation
htscache.c and the two readers in proxy/store.c each spelled out clear-then-strlncatbuff, in binaries that share no code. A helper in htssafe.h states the contract once, and evaluates its arguments once: the macro form expanded refvalue and refvalue_size twice, and (refvalue_size) - 1 would have wrapped to SIZE_MAX had any call site ever passed 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
The cases jumped from the degenerate capacity 1 straight to 8, so a defect confined to small-but-not-degenerate sizes passed: clipping a two-byte destination to the empty string instead of one character. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three places spelled out the same clip-don't-abort copy by hand: the engine's cache reader in
htscache.c, and both the ZIP and ARC readers inproxy/store.c. They live in binaries that share no code, so the duplication could not be caught by the linker, only by noticing it.strclipbuff()inhtssafe.hstates the contract once. It also evaluates its arguments once, which the macro form did not:refvalueandrefvalue_sizewere each expanded twice, and(refvalue_size) - 1would have wrapped toSIZE_MAXif any call site had ever passed zero. No call site does, so that part is hardening rather than a fix.Covered by
-#test=strsafe, mutation-tested against six candidate bugs: an off-by-one clip, a missing terminator, writing nothing when truncating, always reporting a fit, a sign-extended0xffending the copy early, and dropping the last byte. All six fail the test; the restored build passes.