window.__proto__ = <anything> -> "InternalError: too much recursion" / blocks browser

RESOLVED FIXED in Firefox 42

Status

()

Core
JavaScript Engine
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: caitp, Assigned: bz)

Tracking

Trunk
mozilla42
Points:
---

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_3) AppleWebKit/600.6.3 (KHTML, like Gecko) Version/8.0.6 Safari/600.6.3

Steps to reproduce:

assign any value whatsoever to the __proto__ property of the WindowProxy


Actual results:

InternalError: too much recursion / freezes the browser

Subsequent attempts to read __proto__ will block the browser in an infinite loop due to cycles in prototype chain


Expected results:

Trying to set a cyclic value to the prototype should just throw a TypeError and not modify the value

Comment 1

3 years ago
bholley, what is expected WRT DOM and __proto__ nowadays?
Flags: needinfo?(bobbyholley)

Comment 2

3 years ago
Note that I'm working on making the entire global object prototype chain immutable in...I think it's bug 1052139.  Working through an issue in bootstrapping, for sandbox globals, first, tho.
Waldo is currently the most knowledgeable about this subject, I think.
Flags: needinfo?(jwalden+bmo)
Flags: needinfo?(bobbyholley)
Flags: needinfo?
> assign any value whatsoever to the __proto__ property of the WindowProxy
....
> Subsequent attempts to read __proto__ will block the browser in an infinite loop

I just tried this testcase:

  window.__proto__ = {};
  alert(window.__proto__);

and it works just fine both in a current nightly and in Firefox 38.

Now I _can_ reproduce a problem with window.__proto__ = Object.create(window): this allows the set to happen (which it should not) and property accesses then throw "too much recursion"....  But no hang either way.
Flags: needinfo?
(Reporter)

Comment 5

3 years ago
This was broken in Nightly a few days ago, but it seems to be not crashing in 41.0a1 (2015-06-23)
So in terms of the code flow, we end up in js::SetPrototype, obj->hasLazyPrototype() tests true, we land in js::DirectProxyHandler::setPrototype, that lands us back in js::SetPrototype for the inner, and then the loop there doesn't work correctly because it's comparing things up the proto chain to "obj" but "obj" is now the inner.  We need to outerize before the comparison loop.
Created attachment 8625468 [details] [diff] [review]
Fix the proto cycle checking to work correctly for inner/outer globals
Attachment #8625468 - Flags: review?(jwalden+bmo)
Assignee: nobody → bzbarsky
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(jwalden+bmo)

Comment 8

3 years ago
Comment on attachment 8625468 [details] [diff] [review]
Fix the proto cycle checking to work correctly for inner/outer globals

Review of attachment 8625468 [details] [diff] [review]:
-----------------------------------------------------------------

Bah.  There must be a better way to make inner/outer objects not frail and buggy like this, somehow.

::: js/src/jsobj.cpp
@@ +2864,5 @@
>          return result.fail(JSMSG_CANT_SET_PROTO);
>  
> +    /* ES6 9.1.2 step 6 forbids generating cyclical prototype chains. But we
> +       have to do this comparison on the observable outer objects, not on the
> +       possibly-inner object we're setting the proto on. */

/*
 * ...
 * ...
 */

or

// ...
// ...
Attachment #8625468 - Flags: review?(jwalden+bmo) → review+
https://hg.mozilla.org/mozilla-central/rev/57a8f2e1d1e2
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.