Prototype pollution: --constructor / --toString / --__proto__<suffix> stored as own property (CVE-2020-7608 fix bypass)

[zhangjiashuo-cs]

Description

yargs-parser stores option names that collide with Object.prototype properties (constructor, prototype, toString, valueOf, hasOwnProperty, etc., and any __proto__* with a suffix) as own properties on the returned argv object. When the caller subsequently merges argv into another object via Object.assign(target, argv) or {...target, ...argv} (a very common pattern), the attacker-controlled value overwrites those properties on target.

This re-opens the prototype-pollution class of vulnerability that was partially addressed in CVE-2020-7608. The 2020 fix only handled the bare key __proto__; it didn't sanitize the other dangerous keys or the __proto__<suffix> family.

Reproduction (yargs-parser, latest on npm)

const yp = require('yargs-parser');

// 1. constructor / prototype shadowing
const argv = yp(['--constructor', 'malicious']);
console.log(JSON.stringify(argv));         // { _: [], constructor: 'malicious' }

// 2. Realistic exploitation chain
const userConfig = { debug: false, role: 'guest' };
Object.assign(userConfig, argv);
console.log(userConfig.constructor);       // 'malicious'  — was the Object constructor

// 3. Bypassing the CVE-2020-7608 fix
const argv2 = yp(['--__proto__.polluted', 'yes']);
console.log(JSON.stringify(argv2));
// { _: [], "___proto___": { polluted: "yes" } }   — renamed; OK
const argv3 = yp(['--__proto__foo', 'evil']);
console.log(JSON.stringify(argv3));
// { _: [], "__proto__foo": "evil" }   — NOT renamed; attacker can collide with any prototype prop whose name is `__proto__<suffix>`

// 4. Other dangerous keys all pass through
for (const key of ['toString', 'valueOf', 'hasOwnProperty', 'propertyIsEnumerable',
                   'isPrototypeOf', '__defineGetter__', '__lookupGetter__']) {
    const r = yp([`--${key}`, 'evil']);
    console.log(`${key}:`, r[key]);   // 'evil' for all
}

Property that fails

import fc from "fast-check";
import yp from "yargs-parser";

const DANGEROUS = ['constructor', 'prototype', 'toString', 'valueOf',
                   'hasOwnProperty', '__defineGetter__'];

fc.assert(fc.property(
    fc.constantFrom(...DANGEROUS),
    fc.string({minLength:1, maxLength:30}),
    (key, val) => {
        const argv = yp([`--${key}`, val]);
        // Either yargs-parser refuses to use a dangerous key, or
        // it doesn't break Object.assign into a clean object.
        const target = {};
        Object.assign(target, argv);
        return target[key] === Object.prototype[key];   // unchanged
    }
));
// Fails for every DANGEROUS key.

Threat model

The argv object returned by yargs-parser is treated by most callers as a plain config dictionary and is routinely merged into application defaults / config objects. An attacker who controls command-line args (CLI tools accepting flags from untrusted scripts, build steps that read env-driven args, web admin tools that forward query params to internal CLI calls, container orchestrators that pass user args to entrypoints) can:

• Overwrite the constructor of any object obtained via Object.assign(...) from argv, breaking instanceOf checks and exception types downstream.
• Replace toString / valueOf on merged-into objects, which trips JSON.stringify, template interpolation, and equality checks. Many of those happen in security-sensitive contexts (logging, auth checks, role comparisons).
• For applications that walk the merged object with patterns like if (config.constructor === Object) { … }, this is an outright trust bypass.

CVE-2020-7608 was acknowledged as security-relevant for the exact same shape (with --__proto__.x) and was fixed in 18.x — the issue is that the fix's sanitization list was incomplete.

Suggested fix

In src/yargs-parser.ts setArg(), after computing the destination key, reject (or rename) keys that match any of:

• exact: __proto__, constructor, prototype, hasOwnProperty, isPrototypeOf, propertyIsEnumerable, toString, valueOf, toLocaleString, __defineGetter__, __defineSetter__, __lookupGetter__, __lookupSetter__
• prefix: ^__proto__ (i.e. any key starting with __proto__)

The same sanitization should apply in the dot-notation expansion path (setKey), which sanitizeKey already does for bare __proto__ but no other entries.

Environment

• yargs-parser: latest (also reproduces in 21.1.1)
• Node: 20+

[shadowspawn]

This is not prototype pollution, as covered in CVE-2020-7608. Quoting from that entry:

Parsing the argument --foo.__proto__.bar baz adds a bar property with value baz to all objects.

You are clear in your description that yargs is not modifying the underlying prototype.

yargs-parser stores option names that collide with Object.prototype properties (...) as own properties on the returned argv object.

[shadowspawn]

It is very convenient to use ordinary objects in Javascript like a Map, but the keys may collide with prototype properties. You will see that yargs-parser extensively uses Object.create(null) to create an object with no prototype and hence avoid collisions. You can reintroduce the collisions in client code by copying the properties onto a plain object, as in your examples.

I understand your mention of explicit property names, like toString.

I don't understand your example __proto__foo and mentions of __proto__ prefix as being an issue. What can go wrong with that pattern?