-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ASAN error while parsing BJData (Heap-buffer-overflow READ 1) #3492
Comments
FYI @fangq |
Do you mind if I switch the make targets over to using the multi-header version? That would make the stack trace more informative. For example, And btw., the links you provide are inaccessible to us plebs. ;-) Edit: I mean these two: I can download the testcase |
Not at all - please go ahead!
I'll check what needs to be done for this - or whom to ask. |
@nlohmann and @falbrechtskirchinger, I wonder if this is related to the handling of negative sizes as we touched upon in #3475 (comment)? does this only happen for BJData or also for UBJSON? because a negative length is currently static_casted to a size_t, therefore, it can create huge buffer lengths, which can be a problem. I am wondering if you are ok to add a new parse error inside all signed-integer branches of json/include/nlohmann/detail/input/binary_reader.hpp Lines 2016 to 2025 in a8a547d
where we can add a condition in if (JSON_HEDLEY_UNLIKELY(!get_number(input_format, number) || number < 0 )){
return false;
} to discard this, or, more verbosely, for each signed integer if (JSON_HEDLEY_UNLIKELY(!get_number(input_format, number))){
return false;
}
if( number < 0){
return sax->parse_error(chars_read, last_token, parse_error::create(???, chars_read,
exception_message(input_format, "negative sizes are not permitted", "size"), nullptr));
} what do you think? |
I analyzed the issue, and just like #3490 (comment), it is caused by creating invalid SAX events. These events are triggered: <object>
<key key="" />
<array>
<object size="3">
<key key="_ArraySize_" />
<array size="2">
<number_integer val="32" />
<number_integer val="0" />
</array>
<object size="0"> <!-- first error: expected a key here -->
</object>
</array> <!-- second error: parsing an object, but closing an array -->
<key key="" />
<parse_error id="18" token="<end of file>" /> The parser must make sure that
These mismatched events yield undefined behavior in the SAX-DOM parser. In #3498 I propose adding assertions to make this undefined behavior visible. That PR does not fix this issue, but makes it clear that is unrelated to any sanitizer. |
thanks @nlohmann, this is very helpful. I will look into this further. going over the hex dump of the fuzzer data, I spotted a few issues:
here are the issues, and some of my questions:
|
From https://json.nlohmann.me/features/parsing/sax_interface/: // called when an object or array begins or ends, resp. The number of elements is passed (or -1 if not known)
bool start_object(std::size_t elements); Passing |
I see. although https://ubjson.org/type-reference/container-types/#optimized-format |
Exactly. |
…3491,#3492,#3490) (#3500) * Discard optimized containers with negative counts in UBJSON/BJData (#3491,#3492,#3490) * fix msvc error * update unit tests for negative sized containers * use a loop to test 0 ndarray dimension * throw an error when count is negative, merge CHECK_THROW_AS and _WITH with _WITH_AS
Description
OSS-Fuzz reports a buffer overflow when fuzzing with ASAN
Reproduction steps
clusterfuzz-testcase-minimized-parse_bjdata_fuzzer-5698044521218048.zip
Expected vs. actual results
Expected: parse error or valid JSON value returned
Actual: ASAN runtime error
Minimal code example
No response
Error messages
Compiler and operating system
OSS-Fuzz
Library version
develop
Validation
develop
branch is used.The text was updated successfully, but these errors were encountered: