401549 - getBoxObjectFor(elem).firstChild reveals anonymous scrollbars

Status: VERIFIED FIXED

(Core :: Layout, defect, P2)

Description

[Jesse Ruderman]

Web page scripts can get references to anonymous scrollbar elements using getBoxObjectFor(elem).firstChild.

Split from bug 399411.  Note that bug 251197 has the same impact as this one.

Comment 1

[Jesse Ruderman]

Attachment: testcase

Comment 2

[Boris Zbarsky [:bzbarsky]]

I think we should have security checks in the boxobject tree-traversal code....

Then again, the real issue here is that scrollbars are not native-anonymous, right?  Because if they were, the security manager would stop this sort of thing in its tracks.  Perhaps we could just fix that, finally?

Comment 3

[Jesse Ruderman]

Now I get "Permission denied to get property XULElement.tagName".  That's a slight improvement -- I can still get a reference to the element, but I can't do as much with it.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Note that the behavior change from comment 3 has regressed due to quickstubs.  We really need to fix this, imo.

Comment 5

[Damon Sicore (:damons)]

Can we get an owner for this?

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

We must fix bug 475864, so is fixing this really needed?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Ideally, yes...  But it might not be as critical if we fix bug 475864.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

While this is undesirable, can anything bad actually happen with content that gets hold of the anonymous scrollbar? You can't do much with it AFAIK.

Comment 9

[Boris Zbarsky [:bzbarsky]]

That depends entirely on what happens with bug 475864.  Right now you can call any quickstubbed method on it.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

But anyway, I can take this.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yes, but what nastiness can you do by messing with the scrollbar via quickstubbed methods?

Comment 12

[Boris Zbarsky [:bzbarsky]]

Remove its kids, for example?

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

Attachment: testcase, works on trunk too

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

Attachment: possible patch

This is the simple way to fix this. If box object is about to return some
native anonymous element to web page, return null. (BoxObject seems to use null
for error cases.).
The main thing I'm a bit worried about is xulDocument.documentElement.boxObject.firstChild, because that is the default (native anonymous) popupgroup. So I wonder if the nextSibling of that should be actually returned.
Will upload a second patch which does that.

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

Attachment: alternative approach

This is pretty ugly. I'm not sure it is worth to even try to do this.
I think I should wait until bug 475864 is fixed, and see what is left to do
after that.

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

Now that bug 475864 is fixed, the testcase gives a security exception.
Is that good enough for 1.9.1? I think it should be.

Comment 17

[Boris Zbarsky [:bzbarsky]]

That's fine, yeah.

Comment 18

[Olli Pettay [:smaug][[e-mail]]]

Marking this then fixed, also on 1.9.1.

Comment 19

[Aakash Desai [:aakashd]]

verified FIXED using attached testcases on builds:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090721 Minefield/3.6a1pre ID:20090721031556

and

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.2pre) Gecko/20090721 Shiretoko/3.5.2pre ID:20090721075223