-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Use the dom::parser internal loaded_bytes buffer as a tmp padded buffer when realloc_if_needed #1518
Conversation
@bobergj The fuzzer appears to have found an issue, can you have a look? ../include/simdjson/dom/parser-inl.h:112:17: runtime error: null pointer passed as argument 1, which is declared to never be null
310
/usr/include/string.h:44:28: note: nonnull attribute specified here
311
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../include/simdjson/dom/parser-inl.h:112:17 in |
} | ||
_loaded_bytes_capacity = len; | ||
} | ||
std::memcpy(static_cast<void *>(loaded_bytes.get()), buf, len); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be unsafe in the sense that a pointer could be null.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this could indeed happen if the input buffer was empty (len == 0). Fixed in new commit.
It could be that the parse file code path (not modified by this PR) has a similar issue when a zero byte file is parsed, not sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be that the parse file code path (not modified by this PR) has a similar issue when a zero byte file is parsed, not sure.
I will add a test for that specifically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #1519
9455d04
to
10977a4
Compare
This looks pretty good. |
Marked for 1.0 release. |
Merging. |
Use the dom::parser internal loaded_bytes buffer as a tmp padded buffer when realloc_if_needed is true.
Motivation: