Discuss
  • [Harmony proxies] Non-configurable properties: fixed properties VS trap all with invariants enforcement

[Harmony proxies] Non-configurable properties: fixed properties VS trap all with invariants enforcement

# David Bruant (14 years ago)

We may need to switch from the (bicephal) fixed properties model to a "trap all with invariants enforcement" model.

With the current simple membrane example:

var o = {}; var s = makeSimpleMembrane(o); var wrapper = s.wrapper, gate = s.gate;

// ... later, in untrusted code (variable name kept for readability) Object.defineProperty(wrapper, 'a', {value:1, configurable:false});

// ... later, back in trusted code: gate.disable()

// ... later in the same untrusted code which had a reference to the wrapper object. wrapper.a; // 1 in the fixed properties model // throw if the 'get' traps was actually called. // Such a difference is certainly noticeable in all traps.

The problem with the fixed property model is that properties exist on the proxy object itself with no way to control (and prevent in our case) access to them.

# David Bruant (14 years ago)

Le 20/06/2011 17:15, David Bruant a écrit :

Hi,

We may need to switch from the (bicephal) fixed properties model to a "trap all with invariants enforcement" model.

With the current simple membrane example:

var o = {}; var s = makeSimpleMembrane(o); var wrapper = s.wrapper, gate = s.gate;

// ... later, in untrusted code (variable name kept for readability) Object.defineProperty(wrapper, 'a', {value:1, configurable:false});

// ... later, back in trusted code: gate.disable()

// ... later in the same untrusted code which had a reference to the wrapper object.

Actually, it's more convincing if the wrapper is given to another piece of untrusted code, because this code should not have access to what another untrusted code added to the wrapper AFTER disabling. Also, untrusted code could communicate through non-configurable, but writable properties which doesn't sounds good.

# Mark S. Miller (14 years ago)

my compliments. This is an important issue I should have caught but didn't notice. Thanks. I think agree with your conclusion (which I was leaning towards anyway).

# Brendan Eich (14 years ago)

On Jun 20, 2011, at 11:31 AM, Mark S. Miller wrote:

Hi David, my compliments. This is an important issue I should have caught but didn't notice. Thanks. I think agree with your conclusion (which I was leaning towards anyway).

Yeah, ++david.bruant!

# Tom Van Cutsem (14 years ago)

Yes, you made a nice case. In a way I'm glad, as I wasn't too keen on the additional complexity brought about by the bicephal approach (membranes could still be made to work by converting all non-configurable properties into accessors as shown on the wiki page, at the price of giving up fully transparent wrappers)

I guess the way forward is to try and specify precisely what consistency checks a proxy should perform, and how cheap or costly these will turn out to be.

Re. untrusted code communicating via non-configurable, writable properties: I believe this is not a problem specific to proxies with fixed properties, or even proxies in general. As soon as you share a non-frozen object, this kind of communication can happen. But granted: membrane proxies + transparent fixed properties are flawed in this regard.

Cheers, Tom

2011/6/20 Mark S. Miller <erights at google.com>

# Tom Van Cutsem (14 years ago)

It just occurred to me that the security issues you describe re. fixed properties & membranes may not be as bad as we first thought they were.

Because of the recursive wrapping of the membrane code, all of the get/set/value attributes of fixed properties will contain wrappers themselves. So, after revoking a membrane, untrusted code will still have access to the structural information of these properties, but won't be able to do anything useful with its values (they are revoked wrappers themselves). The structural information itself does not convey any useful authority to the untrusted code. The wrapper may still "leak" previously exposed information (e.g. primitives are not wrapped), but the untrusted code could retain access to this information regardless (even if we switched to validating proxies).

So, long story short: I don't think proxies with fixed properties weaken the actual security afforded by membranes, but I fully agree that the fact that information is lingering in revoked proxies is surprising.

Cheers,

Tom

2011/6/21 Tom Van Cutsem <tomvc.be at gmail.com>

# David Bruant (14 years ago)

Le 22/06/2011 10:14, Tom Van Cutsem a écrit :

It just occurred to me that the security issues you describe re. fixed properties & membranes may not be as bad as we first thought they were.

Because of the recursive wrapping of the membrane code, all of the get/set/value attributes of fixed properties will contain wrappers themselves. So, after revoking a membrane, untrusted code will still have access to the structural information of these properties, but won't be able to do anything useful with its values (they are revoked wrappers themselves). The structural information itself does not convey any useful authority to the untrusted code.

It does leak that the property exists (because a value is returned instead of throwing inconditionally). It doesn't sound very dangerous and exploitable, but it's a leak.

The wrapper may still "leak" previously exposed information (e.g. primitives are not wrapped), but the untrusted code could retain access to this information regardless (even if we switched to validating proxies).

The wrapper can also leak previously unexposed information (not for one piece of untrusted code, but at least two). That was the point of the adjustement of my example: ( 0) Untrusted code 1 and 2 have a reference to the wrapper w )

  1. Untrusted code 1 does: Object.defineProperty(w, 'p', {configurable:false, value:1})
  2. Trusted code closes the gate
  3. In Untrusted code 2: 'p' in w === true

Before 1), this 'p' property wasn't exposed to Untrusted code 2. Closing the gate on 2) should prevent everyone (like Untrusted code 2) who hadn't seen 'p' before gate-closing to see it afterward. It is not big, but it allows two pieces of untrusted code to communicate (more than they would if the trap was called and threw right away).

The easy workaround is to hand a different wrappers to different untrusted codes, but duplicating the number of necessary objects to guarantee security is an unfortunate workaround.

# Tom Van Cutsem (14 years ago)

We haven't actually revisited the membrane code in light of the fixed properties strawman.

One change that I can imagine being made, is that a membrane wrapper, when first created, explicitly copies over the non-configurable properties of its target (this is similar to what the "wrap" method does in < strawman:fixed_properties>). After

this initialization, the handler can set a flag that disallows any further properties to be defined via the defineProperty trap. That would disallow untrusted code from defining new properties on the wrapper.

Regardless of whether this would work or not: it sure is introducing a lot of complexity in the already complex membrane code, which is not what I would call progress!

Cheers, Tom

2011/6/22 David Bruant <david.bruant at labri.fr>

# Tom Van Cutsem (14 years ago)

As a follow-up to the fixed properties proposal:

I created a prototype implementation of fixed properties on top of the existing Proxy API. The idea is to wrap an existing proxy handler in a FixedHandler. This FixedHandler does the bookkeeping and checking to ensure that the wrapped handler doesn't misreport non-configurable properties.

Full source code available here: < code.google.com/p/es-lab/source/browse/trunk/src/proxies/FixedHandler.js

I also adapted David Bruant's self-hosted implementation of Arrays using Proxies DavidBruant/ProxyArray to use the above

FixedHandler. Things appear to work out nicely: < tvcutsem/ProxyArray> is a self-hosted implementation of

Arrays using proxies with fixed properties.

"length" appears to be faithfully emulated, but this could use more testing. See tvcutsem/ProxyArray/blob/master/Arrays.html for an

initial testcase.

Cheers, Tom

2011/6/22 Tom Van Cutsem <tomvc.be at gmail.com>

# David Bruant (14 years ago)

Le 03/07/2011 14:59, Tom Van Cutsem a écrit :

As a follow-up to the fixed properties proposal:

I created a prototype implementation of fixed properties on top of the existing Proxy API. The idea is to wrap an existing proxy handler in a FixedHandler. This FixedHandler does the bookkeeping and checking to ensure that the wrapped handler doesn't misreport non-configurable properties.

Full source code available here: code.google.com/p/es-lab/source/browse/trunk/src/proxies/FixedHandler.js

Just to make sure I do not misunderstand your work:

  • Your code relies on current Proxy implementation behavior (FF4&5) which doesn't do any check on the "configurable" attribute for defineProperty and get{Own}PropertyDecsriptor traps
  • The idea is that eventually, by default, any handler would become your wrapped FixedHandler() handler (functionnally, of course; exact implementation would be left at the discretion of implementors)

In your current FixedHandler implementation, all traps of the wrapped handler are called for configurable properties. For non-configurable properties, if already present in this.fixedProps, the "delete", "hasOwn", "has", "get" and "set" traps aren't called while all others are (first instruction of all other traps). This might appear to be an inconsistent behavior to the user. An easy fix could be to change a little bit these traps:

'delete': function(name) { var ret = this.targetHandler'delete'; // allows to throw with personalized message error rather than default decided by the engine.

if (name in this.fixedProps) {
  throw new TypeError(
    "property "+name+" is non-configurable and can't be deleted");
}

return ret;

},

hasOwn: function(name) { var ret = this.targetHandler.hasOwn ? this.targetHandler.hasOwn(name) : // call the trap in all cases if present TrapDefaults.hasOwn.call(this.targetHandler, name); return name in this.fixedProps || ret; // always true if non-configurable },

has: function(name) { var ret = this.targetHandler.has ? this.targetHandler.has(name) : // call the trap in all cases if present TrapDefaults.has.call(this.targetHandler, name); return name in this.fixedProps || ret; // always true if non-configurable },

get: function(rcvr, name) { var ret = this.targetHandler.get ? this.targetHandler.get(rcvr, name) : TrapDefaults.get.call(this.targetHandler, rcvr, name);

var desc = Object.getOwnPropertyDescriptor(this.fixedProps, name);
if (desc !== undefined) {
  if ('value' in desc) {
    return desc.value;
  } else {
    if (desc.get === undefined) { return undefined; }
    return desc.get.call(rcvr);
  }
}

return ret;

},

set: function(rcvr, name, val) { var ret = this.targetHandler.set ? this.targetHandler.set(rcvr, name, val) : TrapDefaults.set.call(this.targetHandler, rcvr, name, val);

var desc = Object.getOwnPropertyDescriptor(this.fixedProps, name);
if (desc !== undefined) {
  if ('writable' in desc) { // fixed data property
    if (desc.writable) {
      desc.value = val;
      this.defineProperty(name, desc);
      return true;
    } else {
      return false;
    }
  } else { // fixed accessor property
    if (desc.set) {
      desc.set.call(rcvr, val);
      return true;
    } else {
      return false;
    }
  }
}
// simulate missing derived trap fall-back behavior
return ret;

},

This way, all traps are called, regardless of configurability and the invariant-complying result is returned in all cases. When it comes to return value, my implementation may surprise the programmer as what he return from his (target)handler doesn't match what is returned in the end. An alternative could be to test if "ret" matches expectations (with harmony:egal, not === of course) and throw if it's not the case.

Unless testing expectation matching, there is barely any overhead in calling the traps (besides the trap themselves, but that's a cost the programmer is ready to pay for configurable properties already) when comparing with your implementation. And it has the advantages of not leaking things the way I've showed it at the beginning of this thread for the membrane use case.

Thanks for the follow-up and the implementation.

# Mark S. Miller (14 years ago)

On Sun, Jul 3, 2011 at 11:07 AM, David Bruant <david.bruant at labri.fr> wrote:

Le 03/07/2011 14:59, Tom Van Cutsem a écrit :

As a follow-up to the fixed properties proposal:

I created a prototype implementation of fixed properties on top of the existing Proxy API. The idea is to wrap an existing proxy handler in a FixedHandler. This FixedHandler does the bookkeeping and checking to ensure that the wrapped handler doesn't misreport non-configurable properties.

Full source code available here: < code.google.com/p/es-lab/source/browse/trunk/src/proxies/FixedHandler.js

Just to make sure I do not misunderstand your work:

  • Your code relies on current Proxy implementation behavior (FF4&5) which doesn't do any check on the "configurable" attribute for defineProperty and get{Own}PropertyDecsriptor traps

I didn't know that. Is there a bug filed on this yet or a test case that demonstrates this bug?

# David Bruant (14 years ago)

Le 03/07/2011 21:32, Mark S. Miller a écrit :

On Sun, Jul 3, 2011 at 11:07 AM, David Bruant <david.bruant at labri.fr <mailto:david.bruant at labri.fr>> wrote:

Le 03/07/2011 14:59, Tom Van Cutsem a écrit :
> As a follow-up to the fixed properties proposal:
>
> I created a prototype implementation of fixed properties on top
of the
> existing Proxy API. The idea is to wrap an existing proxy
handler in a
> FixedHandler. This FixedHandler does the bookkeeping and checking to
> ensure that the wrapped handler doesn't misreport non-configurable
> properties.
>
> Full source code available here:
>
<http://code.google.com/p/es-lab/source/browse/trunk/src/proxies/FixedHandler.js>
Just to make sure I do not misunderstand your work:
- Your code relies on current Proxy implementation behavior (FF4&5)
which doesn't do any check on the "configurable" attribute for
defineProperty and get{Own}PropertyDecsriptor traps

I didn't know that. Is there a bug filed on this yet or a test case that demonstrates this bug?

var h = {getOwnPropertyDescriptor:function(name){return {value:1, configurable:false};}}; var p = Proxy.create(h);

console.log( Object.getOwnPropertyDescriptor(p, 'a') );

I just tried on Firebug (FF5) and it returned {"value":1,"writable":false,"enumerable":false,"configurable":false}. Should have returned configurable:true according to current proposal. There is no bug filed I would be aware of. But I think there is no rush and it can wait until decisions are made at the July TC39 meeting, no? If the some form of the fixed property proposal is accepted, it would replace any bug filed asking to always return true for configurability. Proxies bugs haven't received much attention lately anyway as far as I know.

# Mark S. Miller (14 years ago)

[+stay]

I'd rather have a bug filed whose resolution we can postpone deciding. I'll file it later today. The reason this is more important than it may appear is that there are at least two other proxy implementations in progress that are probably using FF as a reference, perhaps modulo the filed FF proxy bugs as well as the speced semantics.

# David Bruant (14 years ago)

Le 03/07/2011 22:49, Mark S. Miller a écrit :

[+stay]

I'd rather have a bug filed whose resolution we can postpone deciding. I'll file it later today.

Ok. cc me (bruant at enseirb.fr), please.

The reason this is more important than it may appear is that there are at least two other proxy implementations in progress that are probably using FF as a reference, perhaps modulo the filed FF proxy bugs as well as the speced semantics.

oooooh... Are these implementations available somewhere?

# Tom Van Cutsem (14 years ago)

2011/7/3 David Bruant <david.bruant at labri.fr>

Just to make sure I do not misunderstand your work:

  • Your code relies on current Proxy implementation behavior (FF4&5) which doesn't do any check on the "configurable" attribute for defineProperty and get{Own}PropertyDecsriptor traps

Correct. I forgot to mention that I tested this code in Firefox 5.0.

  • The idea is that eventually, by default, any handler would become your wrapped FixedHandler() handler (functionnally, of course; exact implementation would be left at the discretion of implementors)

Yes, at least, if we can come to agree upon its need and whether the overhead is deemed acceptable. For now, FixedHandler.js defines a method "installAsDefault" that replaces the built-in Proxy.create and Proxy.createFunction with functions that always wrap their handler. If Proxy were to be frozen after this change, it would enforce fixed handlers on subsequent scripts.

In your current FixedHandler implementation, all traps of the wrapped handler are called for configurable properties. For non-configurable properties, if already present in this.fixedProps, the "delete", "hasOwn", "has", "get" and "set" traps aren't called while all others are (first instruction of all other traps). This might appear to be an inconsistent behavior to the user. An easy fix could be to change a little bit these traps:

<Tom: removed implementation of the traps>

This way, all traps are called, regardless of configurability and the invariant-complying result is returned in all cases.

That would indeed be more consistent, and would allow e.g. revoked membranes to throw on all operations, regardless of whether the accessed property is fixed.

When it comes to return value, my implementation may surprise the programmer as what he return from his (target)handler doesn't match what is returned in the end. An alternative could be to test if "ret" matches expectations (with harmony:egal, not === of course) and throw if it's not the case.

Yes, I think such a check would be appropriate. In a sense, a trap returning a different value for a {writable:false, configurable: false} property is as much a violation of the spec. as the getOwnPropertyDescriptor trap reporting said property as {configurable: true}.

Unless testing expectation matching, there is barely any overhead in calling the traps (besides the trap themselves, but that's a cost the programmer is ready to pay for configurable properties already) when comparing with your implementation. And it has the advantages of not leaking things the way I've showed it at the beginning of this thread for the membrane use case.

Yes. This prototype implementation, because it continues to forward most operations to its wrapped handler even for fixed properties, is much closer to what you called the "enforcement" model than to the earlier "bicephal" model.

The enforcement turned out to be relatively easy to program in JS itself thanks to the behavior of Object.defineProperty and Object.defineProperties (succeed silently when consistent changes are made, throw otherwise).

# Tom Van Cutsem (14 years ago)

2011/7/3 Mark S. Miller <erights at google.com>

On Sun, Jul 3, 2011 at 11:07 AM, David Bruant <david.bruant at labri.fr>wrote:

  • Your code relies on current Proxy implementation behavior (FF4&5) which doesn't do any check on the "configurable" attribute for defineProperty and get{Own}PropertyDecsriptor traps

I didn't know that. Is there a bug filed on this yet or a test case that demonstrates this bug?

I knew about FF4&5's behavior, but did not yet submit a bug or include a test case because we did not actually reach consensus on this topic yet. That, in fact, is the whole reason why we are having this discussion on fixed properties :-)

Hopefully, the FixedHandler shows that the changes required to proxies to enforce non-configurability are acceptable, so that proxies can support its behavior by default.

# Tom Van Cutsem (14 years ago)

I updated my implementation to fully embrace the "trapping with enforcement" model: all traps on the FixedHandler always forward to the wrapped handler. Some traps will perform consistency checks on the result of the forwarded invocation. Source: < code.google.com/p/es-lab/source/browse/trunk/src/proxies/FixedHandler.js

In particular, the FixedHandler upholds the following invariants:

(In what follows, "fixed properties" is a shorthand for "previously exposed non-configurable properties")

  • getOwnPropertyDescriptor cannot report fixed properties as non-existent
  • getOwnPropertyDescriptor cannot report incompatible changes to a fixed property (e.g. reporting a non-configurable property as configurable, or reporting a non-configurable, non-writable property as writable)
  • getPropertyDescriptor: same constraints as for getOwnPropertyDescriptor. Contrary to the "bicephal" model, in the "enforcement" model, getPropertyDescriptor continues to work fine. The proxy still needs to store information about fixed properties of prototype objects, but the proxy as a whole won't misreport such fixed properties as own rather than inherited properties of the proxy.
  • defineProperty cannot make incompatible changes to the attributes of fixed properties
  • defineProperty cannot reject compatible changes made to the attributes of fixed properties
  • properties returned by the fix() trap are merged with fixed properties, ensuring no incompatible changes can be made when calling freeze, seal, preventExtensions
  • delete cannot report a successful deletion of a fixed property
  • hasOwn cannot report a fixed property as non-existent
  • has cannot report a fixed property as non-existent
  • get cannot report inconsistent values for non-writable fixed data properties
  • set cannot report a successful assignment for non-writable fixed data properties

A few more notes about this implementation:

  • Contrary to the earlier proposed strawman, the defineProperty trap does not need to return a property descriptor anymore, just a boolean success value like delete and set (it previously could return a modified property descriptor so that it could turn data props into accessor props that still triggered the handler. In this updated version, the original handler is always consulted, so the defineProperty trap doesn't need the ability to modify the descriptor.)

  • This implementation of the FixedHandler is entirely compatible with the existing ForwardingHandler. The latter doesn't need to be changed at all. If the ForwardingHandler forwards to a regular, well-behaved object, it will never trigger a violation of the above invariants.

  • A revoked membrane handler wrapped by a FixedHandler will throw on all operations. No fixed properties are leaked.

  • The strawman wiki page does not yet reflect this implementation. Now that I implemented the enforcement model, I do think it's a pure win as compared to the model currently outlined on the wiki page.

I solicit feedback on the above invariant list: did I miss any? Are some superfluous? Is the cost of checking these acceptable?

Cheers, Tom

2011/7/4 Tom Van Cutsem <tomvc.be at gmail.com>

# David Bruant (14 years ago)

Le 04/07/2011 21:50, Tom Van Cutsem a écrit :

I updated my implementation to fully embrace the "trapping with enforcement" model: all traps on the FixedHandler always forward to the wrapped handler. Some traps will perform consistency checks on the result of the forwarded invocation. Source: code.google.com/p/es-lab/source/browse/trunk/src/proxies/FixedHandler.js

In particular, the FixedHandler upholds the following invariants:

(...)

A few more notes about this implementation:

  • Contrary to the earlier proposed strawman, the defineProperty trap does not need to return a property descriptor anymore, just a boolean success value like delete and set (it previously could return a modified property descriptor so that it could turn data props into accessor props that still triggered the handler. In this updated version, the original handler is always consulted, so the defineProperty trap doesn't need the ability to modify the descriptor.)

And the behavior of returning false (silent reject or throw) depends on strict of the calling code?

  • This implementation of the FixedHandler is entirely compatible with the existing ForwardingHandler. The latter doesn't need to be changed at all. If the ForwardingHandler forwards to a regular, well-behaved object, it will never trigger a violation of the above invariants.

Not only will it never trigger a violation, but it will perfectly forward both configurable and non-configurable properties (which required trickery with the "bicephal" model).

  • The strawman wiki page does not yet reflect this implementation. Now that I implemented the enforcement model, I do think it's a pure win as compared to the model currently outlined on the wiki page. :-)

I solicit feedback on the above invariant list: did I miss any? Are some superfluous?

I can't think of anything that would be missing or too much on top of my head.

Is the cost of checking these acceptable?

To answer this question while keeping the goal of having a consistent non-configurable properties mechanism, one has to compare with another alternative that would allow such a mechanism. The only other such mechanism I've heard of was the bicephal model. In this model, updating operations (defineProperty, set, delete) would perform exactly the same invariant checks, so there is no additional cost (besides calling the traps). However, retrieving operations all require one additional test (for consistency) and to throw if necessary, (in the bicephal model, retrieving was "just a lookup"). I find that acceptable. One additional non-code-related cost is that for any trap considered to be added to the spec, it will require attention to make sure invariants can be enforced (hopefully at a small cost).

# Tom Van Cutsem (14 years ago)

2011/7/4 David Bruant <david.bruant at labri.fr>

Le 04/07/2011 21:50, Tom Van Cutsem a écrit :

A few more notes about this implementation:

  • Contrary to the earlier proposed strawman, the defineProperty trap does not need to return a property descriptor anymore, just a boolean success value like delete and set (it previously could return a modified property descriptor so that it could turn data props into accessor props that still triggered the handler. In this updated version, the original handler is always consulted, so the defineProperty trap doesn't need the ability to modify the descriptor.) And the behavior of returning false (silent reject or throw) depends on strict of the calling code?

Yes, that would be the idea, by analogy with 'set' and 'delete'.

  • This implementation of the FixedHandler is entirely compatible with the existing ForwardingHandler. The latter doesn't need to be changed at all. If the ForwardingHandler forwards to a regular, well-behaved object, it will never trigger a violation of the above invariants. Not only will it never trigger a violation, but it will perfectly forward both configurable and non-configurable properties (which required trickery with the "bicephal" model).

Yes. The enforcement model does not introduce any additional work for handlers that emulate or wrap an ES5-conformant object.

They only require the handler writer to pay more attention when the handler is emulating/forwarding to, say, a non-conformant host object. But at that point, presumably, the job of the proxy is to "tame" that host object and to intentionally restore ES invariants, so the additional complexity required to pass all checks is then an explicit goal rather than a nuisance.

  • The strawman wiki page does not yet reflect this implementation. Now that I implemented the enforcement model, I do think it's a pure win as compared to the model currently outlined on the wiki page. :-)

Forgot to mention it earlier: thanks David for your constructive feedback. Iteration is good!

# Sean Eagan (14 years ago)

I like the FixedHandler approach, one remaining concern I have though is that without something like "Proxy.isProxy", object consumers will need to unnecessarily wrap non-proxy objects in order to guarantee invariant maintenance.

# Tom Van Cutsem (14 years ago)

That's why it would be better if the behavior of FixedHandler could become part of the built-in behavior of proxies.

One issue that requires some thought is whether the FixedHandler could ever garbage collect fixed properties. I don't think that is possible, and that would compromise proxy abstractions that emulate objects with an infinite amount of properties that are derived implicitly from some other source. The FixedHandler would store all of those - presumably generated - properties explicitly.

Cheers, Tom

2011/7/5 Sean Eagan <seaneagan1 at gmail.com>

# Tom Van Cutsem (14 years ago)

I updated strawman:fixed_properties

to reflect our most recent thoughts on how proxies could deal with non-configurable properties.

Cheers, Tom

2011/7/4 Tom Van Cutsem <tomvc.be at gmail.com>