Discuss
  • Object.freeze(Object.prototype) VS reality

Object.freeze(Object.prototype) VS reality

# David Bruant (9 years ago)

Half a million times the following meta-exchange happened on es-discuss:

  • if an attacker modifies Object.prototype, then you're doomed in all sorts of ways
  • Don't let anyone modify it. Just do Object.freeze(Object.prototype)!

I've done it on client-side projects with reasonable success. I've just tried on a Node project and lots of dependencies started throwing errors. (I imagine the difference is that in Node, it's easy to create projects with a big tree of dependencies which I haven't done too much on the client side).

I tracked down a few of these errors and they all seem to relate to the override mistake.

  • In jsdom, trying to add a "constructor" property to an object fails because Object.prototype.constructor is configurable: false, writable: false
  • in tough-cookie (which is a dependency of the popular 'request' module), trying to set Cookie.prototype.toString fails because Object.prototype.toString is configurable: false, writable: false

Arguably, they could use Object.defineProperty, but they won't because it's less natural and it'd be absurd to try to fix npm. The Cookie.prototype.toString case is interesting. Of all the methods being added, only toString causes a problem. Using Object.defineProperty for this one would be an awkward inconsistency.

So, we're in a state where no module needs to modify Object.prototype, but I cannot freeze it because the override mistake makes throw any script that tries to set a toString property to an object. Because of the override mistake, either I have to let Object.prototype mutable (depite no module needing it to be mutable) or freeze it first hand and not use popular modules like jsdom or request.

It's obviously possible to replace all built-in props by accessors, of course, but this is a bit ridiculous. Can the override mistake be fixed? I imagine no web compat issues would occur since this change is about throwing less errors.

# Mark S. Miller (9 years ago)

On Thu, Feb 19, 2015 at 9:23 AM, David Bruant <bruant.d at gmail.com> wrote:

It's obviously possible to replace all built-in props by accessors [4], of course, but this is a bit ridiculous.

It is indeed ridiculous. Not fixing this in the ES5 timeframe was my single biggest failure and disappointment as a member of TC39.

For reference, Caja's implementation of the technique described in [4] is at

code.google.com/p/google-caja/source/browse/trunk/src/com/google/caja/ses/repairES5.js#278

As it states, our term for freezing an object so that it does not provoke this problem is "tamper proofing".

Can the override mistake be fixed? I imagine no web compat issues would occur since this change is about throwing less errors.

There was a time when some of the browsers did not suffer from the override mistake, and in so doing, were technically out of conformance with the ES5 spec. During this window, it was clearly still web compatible to fix the override mistake. It was during this window that I raised the issue and argued that it be fixed. I brought this up in meetings several times and never made any progress. Once all browsers suffered equally from the override mistake, it was no longer clearly web compatible to fix it, so I gave up and focused on other things instead.

However, I agree with your suspicion that it actually would still be web compatible to fix this. Unfortunately, the only way to find out at this point is for a browser to try deploying without the override mistake. I don't have much hope.

Instead, I suggest you promote tamper proofing to those audiences to which you currently promote freezing. Though the need for it is indeed ridiculous, it actually works rather well. We've been using it successfully for many years now.

# Andrea Giammarchi (9 years ago)

Just as workaround, if you really need that much to freeze the Object.prototype, you could:

Object
  .getOwnPropertyNames(Object.prototype)
  .filter((name) => !/^constructor|toString|valueIOf$/.test(name))
  .forEach((name) => Object.defineProperty(
    Object.prototype,
    name,
    {value: Object.prototype[name]}
  ))
;
Object.preventExtensions(Object.prototype);

You can eventually fix those 3 properties somehow via accessors.

Not ideal, but it should hopefully make your project work (at least)

# Andrea Giammarchi (9 years ago)

this, and the fact descriptors suffer inheritance which for 3 boolean properties or a method are absolutely not helpful and make the env doomed by Object.prototype.writable = true shenanigans.

Yes, I'd personally +1 all these fixes that made these ES5 features not the easiest one to play with

# Mark S. Miller (9 years ago)

On Thu, Feb 19, 2015 at 9:54 AM, Andrea Giammarchi <andrea.giammarchi at gmail.com> wrote:

Just as workaround, if you really need that much to freeze the Object.prototype, you could:

Your defineProperty call above has no effect. When doing a defineProperty on an existing property, recall that omitting attributes means that the setting of these attributes should not be changed. Omitted attributes only default to false when the property does not already exist.

Interesting that I've seen this bug several times now, by several different authors. Apparently, it is an unanticipated footgun in the ES5 reflection API.

# Luke Scott (9 years ago)

On Feb 19, 2015, at 9:23 AM, David Bruant <bruant.d at gmail.com> wrote:

Half a million times the following meta-exchange happened on es-discuss:

  • if an attacker modifies Object.prototype, then you're doomed in all sorts of ways
  • Don't let anyone modify it. Just do Object.freeze(Object.prototype)!

Depending on your goals you could use Object.seal. It prevents new properties from being added, but allows you to modify existing properties. From there you could selectively re-define the existing properties with Object.defineProperty and set writable:false.

Although I wouldn’t trust the browser anyway and verify everything server side.

# Andrea Giammarchi (9 years ago)

uhm ... have I forgotten a delete or should I have set {value: Object.prototype[name], writable: false, configurable: false} instead ? (enumerable should be preserved as false too)

Yep, actually you got me there, this is a light side effect since usually nobody redefines the Object.prototype, but good to know, I could have fallen hard there.

# Mark S. Miller (9 years ago)

On Thu, Feb 19, 2015 at 10:14 AM, Andrea Giammarchi < andrea.giammarchi at gmail.com> wrote:

uhm ... have I forgotten a delete or should I have set {value: Object.prototype[name], writable: false, configurable: false} instead ? (enumerable should be preserved as false too)

Either would work, but I would do the second. The first technique is not idempotent but the second is. Deleting will fail a second time, but setting a property to its current settings works fine.

# Andri Möll (9 years ago)

this, and the fact descriptors suffer inheritance which for 3 boolean properties or a method are absolutely not helpful and make the env doomed by Object.prototype.writable = true shenanigans.

Umm, those solutions are in opposition. If you seal-freeze-scotch-tape Object.prototype up, no-one's going to add gets, sets and writables to it. Taking inheritance into account in defineProperty won’t then be a problem.

# Andrea Giammarchi (9 years ago)

Yeah, beside the fact whenever you freeze something, and you create a module, or a library, or whatever, you don't know what you are freezing up.

Since priority is arbitrary, if script A sets stuff before script B then you are done.

Also, I use Object.prototype without causing any sort of problems/error since years now, nobody complained, but an Object.freeze upfront would screw the library anyway.

Trust what you load in and avoid XSS from users as much as you can .... you have your problem fixed for most of the cases (and the current state of JS)