tj / node-querystring

querystring parser for node and the browser - supporting nesting (used by Express, Connect, etc)
MIT License
455 stars 66 forks source link

support keys limit size and allow keys #29

Closed fengmk2 closed 11 years ago

fengmk2 commented 12 years ago

Avoid "hash algorithm collision" http://www.ocert.org/advisories/ocert-2011-003.html

tj commented 12 years ago

honestly i dont think anyone will use these settings, you would need to set them in so many places, and those security people even admitted they've never seen it done in practice

fengmk2 commented 12 years ago

@visionmedia If I use connect, I only put these settings into query and bodyParser middlewares if they support options argument.

e.g:

connect()
  .use(connect.query(options))
  .use(connect.bodyParser(options));

If a query string contain too many key-value pairs, and I dont want to set a request body size too small, I think these limit settings would be helpful.

fengmk2 commented 12 years ago

@visionmedia Out security member has just implement the "hash algorithm collision" in V8.

And test success in our nodejs web application using Express.

I will send the test codes to your email later, you can check it out.

tj commented 12 years ago

im not saying it cant be done, there are many other attacks as well, im just saying they never even found it performed in practice. you could also do it with formidable, with json(), with urlencoded(), with querystring(), there are too many vectors to cover