Open Bug 1110083 Opened 10 years ago Updated 2 years ago

Review existing getProto call sites for safety


(Core :: JavaScript Engine, defect)





(Reporter: anba, Unassigned)




(2 files)

Somewhat similar to bug 914830, therefore I've cc'ed everyone from that bug.

Circular prototype chains are going to be possible in ES6. Apparently [1] was closed by removing the finite prototype chain length requirement from [[GetPrototypeOf]] in " Invariants of the Essential Internal Methods" [2]. And that means loops like the one in ObjectMayHaveExtraIndexedProperties (src/jsarray.cpp) need to be rewritten to stop after a certain number of iterations, otherwise the browser hangs. Yuk!


Example from the spec bug report:
var obj1 = {};
var obj2 = {};
var obj3 = {};

var getPrototypeOfCalled = false;
var setPrototypeOfCalled = false;
var p3 = new Proxy(obj3, {
  getPrototypeOf(t) {
    print("getPrototypeOf called");
    if (getPrototypeOfCalled && !setPrototypeOfCalled) {
      setPrototypeOfCalled = true;
      Object.setPrototypeOf(obj2, obj1);
    getPrototypeOfCalled = true;
    return Reflect.getPrototypeOf(t);

Object.setPrototypeOf(obj2, p3);
Object.setPrototypeOf(obj1, obj2);

print(`Object.getPrototypeOf(obj1) === obj2? ${Object.getPrototypeOf(obj1) === obj2}`);
print(`Object.getPrototypeOf(obj2) === obj1? ${Object.getPrototypeOf(obj2) === obj1}`);

Output per ES6 (rev29, December 6, 2014 Draft):
getPrototypeOf called
getPrototypeOf called
Object.getPrototypeOf(obj1) === obj2? true
Object.getPrototypeOf(obj2) === obj1? true
See also discussion from bug 1108895.
Blocks: 1108895
Attached patch protoSplinter Review
I reviewed all callsites to JSObject::getProto that are potentially dangerous, but I am not sure if I got them all. We also need to review the rest of the browser, which can call JS_GetPrototype or js::GetObjectProto.
Attached patch testsSplinter Review
This tests the places where we would otherwise hang.
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.