Repository navigation
Conversation
curl#22767 removed some of the strlen calls, but it wasn't quite sufficient. Curl_cert_hostcheck has a number of places where it implicitly assumes the buffer is zero-terminated: - The check for empty strings assume that, even if the buffer is zero-length, it can be dereferenced. This only works for C strings where there's always a zero byte in there. Replace this with a length check. - The strncmp("*.") call will read past the buffer if the pattern equals "*". This works for C strings because it will stop at the zero byte first. For general buffers, this needs a length check first. The various checks for the first and last character are actually OK, because the caller rejects empty strings first, but I similarly guarded them for completeness. Likewise, I guarded the accesses of hostname in hopes it can eventually be de-C-string-ified, but the call to Curl_host_is_ipnum requires it to be a C string. Update unit1397 to cover these cases. Run each test with a separate-allocated buffer. The two fixed invalid accesses are then caught by ASan and valgrind.
We're not quite sure if it's 1999 yet.
Member
|
Thanks! |
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
2 participants
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.
#22767 removed some of the strlen calls, but it wasn't quite sufficient. Curl_cert_hostcheck has a number of places where it implicitly assumes the buffer is zero-terminated:
The check for empty strings assume that, even if the buffer is zero-length, it can be dereferenced. This only works for C strings where there's always a zero byte in there. Replace this with a length check.
The
strncmp("*.")call will read past the buffer if the pattern equals"*". This works for C strings because it will stop at the zero byte first. For general buffers, this needs a length check first.The various checks for the first and last character are actually OK, because the caller rejects empty strings first, but I similarly guarded them for completeness. Likewise, I guarded the accesses of hostname in hopes it can eventually be de-C-string-ified, but the call to
Curl_host_is_ipnumrequires it to be a C string.Update unit1397 to cover these cases. Run each test with a separate-allocated buffer. The two fixed invalid accesses are then caught by ASan and valgrind.