2

Consider the following simple example of prototype pollution in JavaScript:

function sayHello(name) {
  console.log(`Hi ${name}!`);
}

// Pollute the prototype
({}).__proto__.toString = () => alert('hacked');

// Trigger the exploit
sayHello({});

I was wondering if a similar exploit could be done with Object.fromEntries, so I tested:

function sayHello(name) {
  console.log(`Hi ${name}!`);
}

// Try to pollute the prototype, but doesn't work, even for the same object!
const x = Object.fromEntries([['__proto__', { toString: () => alert('hacked') }]]);

// Try to trigger the exploit, but fail
sayHello({}); // Hi [object Object]
sayHello(x); // Hi [object Object]

The fact that the built-in Object.fromEntries is safe from this exploit is great, I was expecting some protection. However, I thought it would either throw an error or skip setting the __proto__, but to my surprise the __proto__ was actually set!

x.__proto__.toString(); // Exploited!
x.toString(); // Not exploited!!

I was very surprised that Object.fromEntries managed to create an object whose .__proto__.toString is exploited while .toString is not.

So, is this safe?

Can I use Object.fromEntries with unchecked user-supplied data safely?

Titus
  • 22,031
  • 1
  • 23
  • 33
Pedro A
  • 3,989
  • 3
  • 32
  • 56
  • Do you have a need to allow users to write actual functions to feed into your `Object.fromEntries` call? – TKoL Aug 17 '20 at 16:38
  • @TKoL No, only strings and other objects, but I didn't want to rely on an assumption that polluting the prototype with strings is safe... – Pedro A Aug 17 '20 at 16:40
  • but `{ toString: () => alert('hacked') }` isn't a string, `"{ toString: () => alert('hacked') }"` is, and that prorbably won't have the issue you're talking about – TKoL Aug 17 '20 at 16:41
  • If you allow for any of the values in the arrays to be an IIFE, the exploit can be done using something like this: `const x = Object.fromEntries([['prop', (() => Object.prototype.toString = alert('exploited'))()]]);` – Titus Aug 17 '20 at 16:48
  • @Titus IIFEs are not applicable to my case, users can just pass an arbitrary value, but not code to be eval-ed. – Pedro A Aug 17 '20 at 17:23

1 Answers1

3

Can I use Object.fromEntries with unchecked user-supplied data safely?

Yes, it will never modify Object.prototype by building an object.

I was very surprised that Object.fromEntries managed to create an object whose .__proto__.toString is exploited while .toString is not.

There's nothing special about .__proto__ here, it's just a getter/setter property on Object.prototype, similar to hasOwnProperty or isPrototypeOf.

You will notice that Object.fromEntries does build an object with an own .__proto__ property, and that x.__proto__ !== Object.prototype (although still Object.getPrototypeOf(x) === Object.prototype). The inherited property is shadowed.

Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Awesome! Thank you very much! Does this mean I could even mimic this behavior by calling `Object.defineProperty` myself instead of relying on `Object.fromEntries`? If yes, can you please show a quick example on how to use `Object.defineProperty` safely this way? – Pedro A Aug 17 '20 at 17:42
  • 1
    @PedroA Yes, sure, `Object.defineProperty({}, "__proto__", {value: …, enumerable: true, configurable: true, writable: true})` – Bergi Aug 17 '20 at 18:21
  • Awesome, your answer was exactly what I was looking for. Thank you very much!! – Pedro A Aug 17 '20 at 18:25
  • One last quick question, I've noticed that libraries such as lodash and [dot-prop](https://github.com/sindresorhus/dot-prop#path) ignore setting values for `__proto__` for security reasons... Do you see any reason why they can't just use `Object.defineProperty` instead, as you posted in the last comment? – Pedro A Aug 17 '20 at 18:28
  • 1
    I suppose they could, but it might mean that for symmetry they would need to ignore inherited properties for `get`ting values as well and just don't want this. Also, "for security reasons" you would want to avoid creating such objects in the first place, as *other* code (even native methods like `Object.assign`) might misbehave with objects containing own `__proto__` properties. – Bergi Aug 17 '20 at 18:38