htsAddLink walks back from an empty codebase, one byte before the buffer#767
Merged
Conversation
Same idiom as the lienrelatif() underflow fixed in #729: the trim that walks back to the last '/' starts at codebase + strlen(codebase) - 1, which is codebase - 1 when the string is empty, and the loop dereferences it before a > codebase stops it. Unlike #729 there is no reachable empty value. codebase is copied from a recorded link's fil, and no hts_record_link() call site can supply an empty one: every fil is either a literal seed or an ident_url_absolute() / ident_url_relatif() success return, and fil_simplifie() restores "/" or "./" rather than leaving a path empty. The guard goes in anyway, and -#test=addlink drives the walk directly: under the sanitize job's ASan+UBSan build the test fails on the unfixed walk and passes with the guard. Its second case pins the ordinary trim, which the guard leaves alone. Signed-off-by: Xavier Roche <roche@httrack.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
For an ordinary relative link ident_url_relatif() re-derives the directory
from the path it is handed, so deleting the trim outright left the two
existing cases green. A query-only link ("?x=1") copies that path whole, and
does catch it.
Signed-off-by: Xavier Roche <roche@httrack.com>
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
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.
Same idiom as the
lienrelatif()underflow fixed in #729. The trim that walks back to the last/starts atcodebase + strlen(codebase) - 1, which iscodebase - 1when the string is empty, and the loop dereferences it beforea > codebasecan stop it.Unlike #729 there is no reachable empty value, and I checked rather than assumed:
codebaseis copied from a recorded link'sfil, and nohts_record_link()call site can supply an empty one. Everyfilis either a literal seed or anident_url_absolute()/ident_url_relatif()success return, andfil_simplifie()restores"/"or"./"rather than leaving a path empty. A probe over 37 hostile URL shapes produced no empty result, with a seeded empty control to show the probe could see one.The guard goes in anyway, and
-#test=addlinkdrives the walk directly, so this is a regression net rather than a characterization test: in thesanitizejob's ASan+UBSan build the test fails on the unfixed walk and passes with the guard. Two further cases pin the ordinary trim, which the guard leaves alone. One of them uses a query-only link, because that is the shape where the trim matters at all: for an ordinary relative linkident_url_relatif()re-derives the directory itself, and deleting the trim outright goes unnoticed.Closes #730