Closed DrLoopFall closed 1 year ago
Testing this today...
a) Good catch! How did you even find this?? b) Not sure if we should fix it... because:
a) Good catch! How did you even find this??
Was checking Node's querystring.parse
, and found that it returns an object with a null prototype.
params will fall under the same issue I suspect (and would cost more bytes to fix)
The params
object already has a null prototype, match.groups
is an object with a null prototype.
For consistency, we can also make prototype of the fallback object null request.params = match.groups || { __proto: null }
I think the worse side effect of fixing it is that a thing that looks like an object (query, params, etc) actually fails to have the properties/methods of a real object.
Not fixing would result in objects having arrays instead of the expected object's properties/methods.
e.g. req.toString()
would throw error.
Here are a few query parser libraries behavior,
querystring
-> null prototype.query-string
-> null prototype.qs
-> skips keys in Object.prototypequerystringify
-> skips keys in Object.prototypeThis bug is not much likely to cause security issues. If we want we can leave it as it is.
Bug
URL
...?toString=value
results in query{ toString: [ <toString function>, "value" ] }
.Solutions
Object.create(null)
or{ __proto__: null }
.Object.prototype
in query.This would be a breaking change.