Discuss
  • The Invariants of the Essential Methods are false in case of reentrancy

The Invariants of the Essential Methods are false in case of reentrancy

# Claude Pache (8 years ago)

Test case:

var results = []

var observe = function (method, object, ...args) {
    results.push({
        method: method
      , object: object
      , args: args
      , output: Reflect[method](object, ...args)
    })
}

var obj = { get x() { 
    Object.defineProperty(this, 'x', { value: 2, configurable: false, writable: false })
    observe('getOwnPropertyDescriptor', this, 'x')
    return 1
} }

observe('get', obj, 'x')

results[0]
// { method: "getOwnPropertyDescriptor", object: obj, args: [ "x" ], output: { value: 2, writable: false, enumerable: true, configurable: false } }
// `obj` is observed to have a nonconfigurable, nonwritable property "x" with value `2`.

results[1] // { method: "get", object: obj, args: [ "x" ], output: 1 }
// Then, `obj.[[Get]]("x") is observed to return `1`.

Not sure if it is worrisome. What is sure, the situation could become worse with fanciful proxies.

―Claude

# Raul-Sebastian Mihăilă (8 years ago)

In depends on what observing means. In this "inception" kind of observing, you could argue that the property hasn't yet been observed because [[Get]] was called before.

# Tom Van Cutsem (8 years ago)

I don't think it's too worrisome because of the following reasoning:

Any code prior to the first invocation of obj.x (let's call this the "outer code") will have observed the property as configurable & writable, so having obj.x evaluate to 1 is a valid outcome.

Any code that runs synchronously between the call to Object.defineProperty and the return 1 statement (let's call this the "inner code"), will consistently observe obj.x as non-configurable,non-writable and will always read 2.

A worrisome scenario would be when the inner code computes results based on the new value of x that are also accessible to the outer code, and the outer code somehow expects that value to be consistent with the old value of x. However, it's unclear that this bug is really due to the violation of an ES invariant.

I'm not sure if proxies would amplify the problem. Actually, the invariant assertions in the Proxy internal methods were designed precisely to avoid such bugs, which is why all the assertions trigger when the trap has finished executing and the proxy can no longer influence the outcome of the intercepted operation. The bug described above would be avoided if the internal methods of Ordinary ES objects would apply the same kind of invariant checks as proxies do, checking the result of the [[Get]] operation with the property's current attribute state. However, I suspect that would probably be a bridge too far in terms of cost vs payoff.

, Tom

2016-08-10 17:51 GMT+02:00 Claude Pache <claude.pache at gmail.com>:

# Claude Pache (8 years ago)

Le 11 août 2016 à 13:14, Tom Van Cutsem <tomvc.be at gmail.com> a écrit :

I don't think it's too worrisome because of the following reasoning:

Any code prior to the first invocation of obj.x (let's call this the "outer code") will have observed the property as configurable & writable, so having obj.x evaluate to 1 is a valid outcome.

Any code that runs synchronously between the call to Object.defineProperty and the return 1 statement (let's call this the "inner code"), will consistently observe obj.x as non-configurable,non-writable and will always read 2.

A worrisome scenario would be when the inner code computes results based on the new value of x that are also accessible to the outer code, and the outer code somehow expects that value to be consistent with the old value of x. However, it's unclear that this bug is really due to the violation of an ES invariant.

I'm not sure if proxies would amplify the problem. Actually, the invariant assertions in the Proxy internal methods were designed precisely to avoid such bugs, which is why all the assertions trigger when the trap has finished executing and the proxy can no longer influence the outcome of the intercepted operation. The bug described above would be avoided if the internal methods of Ordinary ES objects would apply the same kind of invariant checks as proxies do, checking the result of the [[Get]] operation with the property's current attribute state. However, I suspect that would probably be a bridge too far in terms of cost vs payoff.

If we want to avoid the "bug" I mentioned, the invariant checks for proxies have to be done very carefully in order to avoid some really weird (and most surely implausible) edge cases as demonstrated by the scenario below. The key fact is that, while the traps of the proxy may behave regularly, the target might be itself an exotic object that manifests weird behaviours.

Let P be a Proxy, T its target object, and let's invoke P.[GetOwnProperty]. tc39.github.io/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-getownproperty-p, tc39.github.io/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-getownproperty-p

The following scenario is possible:

  • step 8: P's getOwnPropertyDescriptor trap is invoked on property "x". That trap exists but simply returns T.[GetOwnProperty]
  • step 10: T.[GetOwnProperty] returns naturally the same descriptor.
  • step 12 (or 11c): T.[IsExtensible] is invoked. But it happens that T is a strange exotic object (e.g. a proxy) with a weird [[IsExtensible]] method that does the following:
    • if it is extensible,
      1. it deletes its property "x";
      2. it makes itself nonextensible;
      3. it publishes somewhere that P is nonextensible and has no property "x" (e.g., by using the observe function of my original message);
  • Following steps: Checks are made based on results of steps 10 and 12. Naturally, given the result of step 10, all checks pass.

As a consequence, the property "x" is observed as present on P, while it was previously observed as absent on nonextensible P.

(A solution would be to invoke T.[IsExtensible] only when we know in advance that we will throw in case T is nonextensible. But it is probably not worth to worry for that.)