Possible security hole #397

Closed
chjj opened this Issue Oct 23, 2011 · 1 comment

Comments

Projects
None yet
2 participants

chjj commented Oct 23, 2011

I was just aimlessly browsing the code a little bit and think I might've spotted something.

A line in the body parser middleware reads:

var parser = exports.parse[mime(req)];

mime returns the content-type and a property of exports.parse is accessed using the mime type. Since exports.parse inherits from Object, an attacker could select any property from Object.prototype. Someone could easily send a content type of __lookupSetter__ or something else on the object prototype. The parser variable would be assigned to Object.prototype.__lookupSetter__, called with a context of the global object, with the request body as its first parameter.

All someone would have to do is send a request that looks something like:

POST / HTTP/1.1
Content-Type: __lookupSetter__

hello world
$ curl -X POST -d "hello world" -H "Content-Type: __lookupGetter__" http://127.0.0.1/

At the very least, it could set req.body to undefined and cause some unexpected errors. It could probably do something worse depending on the environment. I guess a simple hasOwnProperty check would fix this.

(edit: In my original tests, Object.prototype.valueOf called with the global object as the context seemed to return the global object in the node repl. I can't seem to reproduce it with connect. That might be a pretty serious issue if req.body was getting set to the global object, but it seems alright.)

Contributor

jonathanong commented Sep 15, 2013

that line in bodyparser no longer exists

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment