Jesse Ruderman Reporter
	Description • 17 years ago

This crashes Firefox (Mac trunk debug, just updated):

function boom()
{
  var cdp = document.body.compareDocumentPosition;
  var ew = eval(window); // This creates a cross-origin wrapper!!!
  window.__proto__ = ew;
  cdp(null);
}

	Jesse Ruderman Reporter
	Comment 1 • 17 years ago

I guess this is creating a __proto__ cycle that's undetected due to the XOW, so the bug is in the __proto__ setting code for not detecting the cycle.  Why does eval(window) create a XOW, though?

	Jesse Ruderman Reporter
	Comment 2 • 17 years ago

Oh, the eval isn't needed.  |window| itself is a XOW and breaks __proto__ cycle prevention.

(Why are |window| and |document| XOWs?)

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 4 • 17 years ago

(In reply to comment #2)
> (Why are |window| and |document| XOWs?)

Because they can change principals, consider:

w = window.open(); // w is same-origin.
w.location = 'http://www.google.com'; // w is now not same-origin.

We create an XOW to ensure that we always do the right security checks.

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 5 • 17 years ago

This should fix things: now the XOW does the cycle check, unwrapping along the way. This is a little bit complicated, since doing:

  window.__proto__ = null
  window.__proto__ = window

Does not actually create a cycle (since the first assignment disables __proto__ setting).

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 6 • 17 years ago

Oops, used the wrong variable in the JS_GetPrototype in the loop condition.

	Brendan Eich [:brendan]
	Comment 7 • 17 years ago

Comment on attachment 275006 [details] [diff] [review]
Fix, v1.1

>+  JSObject *prototype = nsnull; // Initialize this to quiet GCC.

s/prototype/proto/ to match other uses of the shorthand, for brevity, and to keep the f.prototype property of function objects distant?

>+    // Ensure that this __proto__ setting didn't create a cycle. The JS
>+    // engine tries to do this, but XOWs confuse it, here we deal with
>+    // them by unwrapping each step up the prototype chain.

Full stop after "confuse it." Perhaps the new sentence starting after that should begin "So here we deal...".

r=me, nits picked.

/be

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 9 • 17 years ago

Fix checked into trunk.

	Carsten Book [:Tomcat]
	Comment 10 • 17 years ago

verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b3pre) Gecko/2007123104 Minefield/3.0b3pre and the testcase. No crash on testcase - changing to Verified fixed.