[security] Prevent overriding of build-in properties by default #19
Conversation
.travis.yml
Outdated
| - "0.8" | ||
| before_install: | ||
| - 'if [ "${TRAVIS_NODE_VERSION}" == "0.8" ]; then npm install -g npm@2.14.15; fi' | ||
| - "5" |
There was a problem hiding this comment.
Old and no longer supported, I'd remove it.
| // methods like `toString` or __proto__ are not overriden by malicious | ||
| // querystrings. | ||
| // | ||
| if (key in result) continue; |
There was a problem hiding this comment.
We should be safe even without this, as all of them can be safely overridden, with the exception of __proto__ but setting it to a string has no effect.
That said I'm also ok with this.
There was a problem hiding this comment.
Without the key in result check, the test will fail because toString() can be overridden.
There was a problem hiding this comment.
Yes, that's my point, if your query string has a parameter called toString the expected behaviour is probably to find that parameter in the parsed object.
It overrides the toString inherited by Object.prototype but that another issue. I mean ideally the parsed object should not inherit from Object.prototype at all but we can't use Object.create(null) due to old browsers support.
|
This needs a major version bump. |
Updated 'querystringify' to fix [WS-2018-0588 vulnerability](unshiftio/querystringify#19)
See the link below for more details: unshiftio/querystringify#19
Currently, it's possible to override built-in properties of the resulting query string object if a malicious string is inserted in the query string, for example
?toString&will set thetoStringmethod of the object to thetrueboolean. This patch ensures that we never override built-in properties or previously set properties.