sindresorhus / sindresorhus/query-string
`parse()` treats `types` keys inherited from `Object.prototype` as custom types
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 6.9k
- Forks
- 455
- PR merge metrics
- No merged PRs in 30d
Description
Pull requests appear to be disabled on this repo, so filing as an issue. I have a tested one-line fix on a branch and am happy to hand it over in whatever form is useful: https://github.com/MFA-G/query-string/tree/fix/types-prototype-keys
Problem
parse() indexes the types map with keys taken straight from the query string:
returnValue[key] = parseValue(value, options, options.types[key]);
The default is Object.create(null), but that default is discarded as soon as a caller passes their own map — which is the only way the option is ever used. A plain {} inherits Object.prototype, so a query key that collides with a prototype member resolves to the inherited value instead of undefined, and parseValue treats it as a custom type function.
Reproduction
query-string 9.5.1, Node 22:
import queryString from "query-string";
queryString.parse("toString=1&a=2", {types: {}});
//=> {a: "2", toString: "[object Undefined]"} expected toString: "1"
queryString.parse("toString[]=1&toString[]=2", {arrayFormat: "bracket", types: {}});
//=> {toString: ["[object Undefined]", "[object Undefined]"]}
queryString.parse("constructor=1", {types: {}});
//=> {constructor: ["1"]} expected constructor: "1"
valueOf and hasOwnProperty do not merely corrupt the value, they throw — so a query string can crash a parse that has nothing to do with those keys:
queryString.parse("valueOf=hello", {types: {}});
//=> TypeError: Cannot convert undefined or null to object
queryString.parse("hasOwnProperty=x", {types: {}});
//=> TypeError: Cannot convert undefined or null to object
Since query keys are usually attacker-controlled, any parse call with a caller-supplied types map has a small DoS / value-corruption surface. Passing Object.create(null) as types avoids all of it, but nothing in the docs suggests that is required.
Suggested fix
Normalize options.types to a null-prototype object right after the option merge, so only own keys are consulted:
options.types = {__proto__: null, ...options.types};
This preserves the documented default and every documented use of the option; the only behavior that changes is the accidental prototype lookup. Callers already passing Object.create(null) are unaffected.
I have a regression test for toString / valueOf / hasOwnProperty / constructor plus the array-format path on the branch above; npm test is green (191 passed, plus the pre-existing known failure).
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at the parse() path where options.types[key] is passed to parseValue, and review the supplied reproduction cases for prototype keys and array formats. Run npm test and verify regression coverage for toString, valueOf, hasOwnProperty, and constructor while preserving normal custom types behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- security
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100