Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

It seems like this vulnerability is yet another prototype pollution vulnerability.

There was a TC39 proposal a few years ago [0] that proposed to block the getting/setting of object prototypes using the bracket notation, which would have prevented this vulnerability.

At the moment, every single get/set with a square bracket, which uses untrusted data, needs to do some manual check to see whether variables contain "bad" keys like `__proto__`, `prototype,` `constructor`, and so on. This is incredibly annoying, and doesn't really fix the issue. It's possible also to freeze an object's prototype, but that causes other issues. It's also possible to use Object.create(null), and Object.hasOwn (also known as Object.prototype.hasOwnProperty), but again, this does not scale because it has to be done _every single time_.

Maybe it's time to revisit this from a language perspective, instead of continuous bandaid fixes for this language-specific vulnerability (a similar language-specific vulnerability exists in Python called class pollution, but it's .. extremely uncommon).

[0]: https://github.com/tc39/proposal-symbol-proto





Note however, that proposal does not cover some other types of prototype pollution, such as:

  > let config = {};
  > Object.assign(config, JSON.parse('{"__proto__": {"isAdmin": true}}'));
  console.log({}.isAdmin); // true!
or:

  > console.log({}['constructor'] ? {}['constructor']('THIS MUST NOT BE EXPOSED') :  'pub')
  [String: 'THIS MUST NOT BE EXPOSED']

reply


For the first case, it doesn't work because Object.assign() does not copy the prototype or non-enumerable properties, see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Refe...

  > let config = {};
  > Object.assign(config, JSON.parse('{"__proto__": {"isAdmin": true}}'));
  console.log({}.isAdmin); // undefined
That being said, it _will_ happen if you use your own merge() function like the TC-39 proposal demonstrates, but its because you are using the [] syntax to implement it which can affect __proto__

Side note, JSON.parse() also doesn't let you set the actual prototype:

  > JSON.parse('{"__proto__": {"isAdmin": true}}')
  { ['__proto__']: { isAdmin: true } }
  > JSON.parse('{"__proto__": {"isAdmin": true}}').isAdmin
  undefined
A normal JS object can do it, but of course that isn't attacker controlled unless you are using `eval()`, in which case the battle is lost anyway.

  > {"__proto__": {"isAdmin": true}}.isAdmin
  true
But even if JSON itself doesn't set the actual prototype, combining it with a user-written merge() function that copies __proto__ will indeed pollute.

  > Object.entries(JSON.parse('{"__proto__": {"isAdmin": true}}')).reduce((o, [k, v]) => o[k] = v, {}).isAdmin
  true

reply


That was a typo, yeah. It should be

  console.log(config.isAdmin); // true!

reply


On Node.js there are some hardening flags like --disable-proto=throw and --frozen-intrinsics to mitigate/crash on prototype pollution, and to prevent dynamic evals with --disallow-code-generation-from-strings - however, Vercel doesn't seem to support custom node runtime options.

reply




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: