viggox / rapidjson

Automatically exported from code.google.com/p/rapidjson
MIT License
0 stars 0 forks source link

rapidjson accessing memory it doesn't own #108

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
In FindMember(const Ch* name), `name` is accessed directly without regards to 
the length of `name` itself. `name` is provided by the user, so we can't assume 
anything about it. 

This is not causing bugs in production, but I think it's still bad practice. 
Enabling Guard Malloc 
(https://developer.apple.com/library/mac/documentation/performance/conceptual/ma
nagingmemory/articles/MallocDebug.html) on Mac OS X is catching this. 

Also, I'm not sure what this access is looking for. If the `name` is not 
0-terminated, it that such a bad thing?

Original issue reported on code.google.com by guillaum...@gmail.com on 21 Apr 2014 at 5:09

GoogleCodeExporter commented 8 years ago
Forgot to specify : document.h line 271. 

Original comment by guillaum...@gmail.com on 21 Apr 2014 at 5:10

GoogleCodeExporter commented 8 years ago
Thanks for reporting this issue.

I agree that this is a bug and can indeed lead to problems in production, e.g. 
in cases where 'name' is dynamically allocated and (significantly) shorter than 
the name of the (longest) member element in the JSON object.

Since the existing member names may be coming from the outside world, e.g. when 
using 'FindMember' to validate the contents of the received JSON object, this 
could at least lead to a DoS issue in some cases.

You can find a patch in my personal/unofficial fork at GitHub at
  https://github.com/pah/rapidjson/commits/fixes/108
  https://github.com/pah/rapidjson/commit/6558b8fa
(append .patch or .diff to download the raw contents)

Original comment by philipp....@gmail.com on 23 Apr 2014 at 3:55

GoogleCodeExporter commented 8 years ago
Fixed in 
https://github.com/miloyip/rapidjson/commit/02673bec74967dcd108be1a77d9e8841b38d
1aa0

Original comment by milo...@gmail.com on 20 Jun 2014 at 11:17

GoogleCodeExporter commented 8 years ago
Issue 109 has been merged into this issue.

Original comment by milo...@gmail.com on 20 Jun 2014 at 11:17