328012 - "Permission denied to get property ChromeWindow.PropertyIterator"

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Jesse Ruderman]

To reproduce:
  javascript:try { for (i in focus.prototype.toString); } catch(e){alert(e);}

Result:
  Permission denied to get property ChromeWindow.PropertyIterator

Expected:
  No error thrown?

I don't think focus.prototype.toString is a ChromeWindow, so I don't understand why I'm not being allowed to iterate over its properties.

I'm testing using a Mac trunk debug build.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

This is the return of the PropertyIterator (also see bug 323833). We're again trying to create the property iterator to iterate over the toString function, using the toString function's object as the parent. Note, however, that the function was created with a parent leading up to the hidden (chrome) window. Then, we try to resolve the PropertyIterator constructor using its parent as a starting point, and find it on the ChromeWindow, except that we're content, so the ssm throws.

I think there might be a hackish solution: to create the property iterator with the parent being the current global object and stick the iteree into the parent slot once the iterator's been created. I'm not sure how happy I am with that solution (note: this solution also breaks if we ever do a security check on the property iterator, which I don't believe happens).

Comment 2

[Boris Zbarsky [:bzbarsky]]

So how come the focus function's parent chain goes to the hidden window?  Is it because it's actually on Window.prototype?

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Proposed fix

This is my proposed fix (which I found lying around in one of my trees!). To be clear, the function's parent chain doesn't point to the chrome window. Instead, first note that the function, when accessed, is cloned. Resolving "prototype" on the clone will create a new object (still parented at the original content window) to return, but whose __proto__ points at the original function's prototype object (which is parented to the chrome window. Since toString exists on the original prototype object, we don't create a new toString function for the clone's prototype, and so accessing focus.prototype.toString finds the chrome-parented toString.

This patch works by first attempting to resolve the PropertyIterator constructor on the regular window (using the content led scope chain as a starting point) instead of starting at the parent (chrome-parented toString).

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 215953 [details] [diff] [review]
Proposed fix

r=me, glad you found this! I'd forgotten about it.

/be

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into trunk. I'm changing the component to reflect where the bug was fixed.

Comment 6

[Erik Fabert]

Comment on attachment 215953 [details] [diff] [review]
Proposed fix

>+                propobj->slots[JSSLOT_PARENT] = obj;

MSVC complains about this line:
jsinterp.c(2484) : warning C4047: '=' : 'jsval' differs in levels of indirection from 'JSObject *'

obj was set in line 2436:
  obj = JSVAL_TO_OBJECT(sp[i]);

Shouldn't slots[JSSLOT_PARENT] always be a jsval?

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

For the record, brendan checked in the fix for the warning.

Comment 8

[Jay Patel [:jay]]

Comment on attachment 215953 [details] [diff] [review]
Proposed fix

Please check in promptly on the 1.8.0 branch.  Thanks!

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into the 1.8 branches.

Comment 10

[Bob Clary [:bc] (inactive)]

Checking in js1_5/Regress/regress-328012.js;
/cvsroot/mozilla/js/tests/js1_5/Regress/regress-328012.js,v  <--  regress-328012.js
initial revision: 1.1
done

Comment 11

[Bob Clary [:bc] (inactive)]

verified fixed 20060425 browser win/mac/linux 1.8.0.4, 1.8.1, 1.9a1

Comment 12

[Bob Clary [:bc] (inactive)]

Attachment: ChromeWindow errors

I am still getting a number of ChromeWindow related errors on the trunk. Blake, what would you like me to do with these (and no, inserting them into body cavities is not an option) ?

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

Bob, could you open new a new bug on those errors, please?

Comment 14

[Bob Clary [:bc] (inactive)]

(In reply to comment #13)
> Bob, could you open new a new bug on those errors, please?
> 
Sure, I don't seem to be able to reproduce by hand at the moment. I'll file if they still show up in the next run.