Skip to content
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

Fix out-of-bounds read in document.h #7

Closed
wants to merge 1 commit into from
Closed

Fix out-of-bounds read in document.h #7

wants to merge 1 commit into from

Conversation

rsolomakhin
Copy link

FindMember() compares length of "name" and "member->name.data_.s" via "name[member->name.data_.s.length] == '\0'", which can lead to reading past the length of "name". This patch stores the length of "name" (as determined by position of '\0') into "name_length" and changes the length comparison to be "name_length == "member->name.data_.s.length".

FindMember() compares length of "name" and "member->name.data_.s" via "name[member->name.data_.s.length] == '\0'", which can lead to reading past the length of "name". This patch stores the length of "name" (as determined by position of '\0') into "name_length" and changes the length comparison to be "name_length == "member->name.data_.s.length".
@pah
Copy link
Contributor

pah commented Jun 17, 2014

This has been reported as upstream issue 108 and a fix similar to yours is part of my for-upstream branch in my GitHub fork, see also #1.

Compared to the fix in pah/rapidjson@f86af8c2, your patch does not handle different character types. You should use the internal::StrLen function to precompute the length of the name parameter.

@rsolomakhin
Copy link
Author

Yes, your fix is better. I see that you've started to work with the developer in #1. I will close this pull request and issue and look forward to your patch being accepted. Good luck!

miloyip added a commit that referenced this pull request Jun 20, 2014
The original FindMember() may access out-of-bound of the 'const char*
name' parameter.
This commit firstly follows
pah/rapidjson@f86af8c

However, this must incur an StrLen() for name. A better API is by using
Value as the name, which provides the length of string internally. So a
set of new API are added:

operator[](const GenericValue& name)
FindMember(const GenericValue& name)
RemoveMember(const GenericValue& name)

During refactoring, it also adds an API:

RemoveMember(MemberIterator m)

which can be used for other purpose, such as removing a member while
iterating an object.

Fixes #7
@xinthose xinthose mentioned this pull request Aug 19, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants