fix stack underflow in ippAddStringfv/ippSetStringfv truncation - #1664
Open
aizu-m wants to merge 1 commit into
Open
fix stack underflow in ippAddStringfv/ippSetStringfv truncation#1664aizu-m wants to merge 1 commit into
aizu-m wants to merge 1 commit into
Conversation
When trimming an over-long value to the tag maximum at a UTF-8 boundary, the inner loop skips continuation bytes bounded by bufptr > buffer but the outer decrement after it does not, so a value made only of continuation bytes walks bufptr to buffer - 1 and *bufptr = '\0' writes one byte below the stack buffer. Guard the outer decrement like the inner loop already does, in both functions. Signed-off-by: Aizal Khan <aizumusheer2@gmail.com>
Member
|
Investigating... (Please use your GPG key to sign commits for this repository...) |
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.
Turned up while fuzzing the string constructors with malformed UTF-8.
AddressSanitizer, testipp calling
ippAddStringf()with a value that is all0x80continuation bytes and a length-limited tag:When an over-long value is trimmed to the tag maximum at a UTF-8 boundary, the inner loop that skips continuation bytes is bounded by
bufptr > buffer, but the outerbufptr --that follows it is not. A value whose firstmax_bytesbytes are all continuation bytes (no lead byte) walksbufptrdown tobuffer, the outer decrement then drops it tobuffer - 1, thewhile (bufptr > bufmax)exits, and*bufptr = '\0'stores one byte below the array. As a side effect the value is also left untrimmed.ippSetStringfvcarries the same block, so both get the guard. It only mirrors what the inner loop already does. Added a testipp case that fails before the change (value not trimmed) and passes after.