Skip to content

parse() treats types keys inherited from Object.prototype as custom types #421

Description

@MFA-G

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).

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions