670317 - "ASSERTION: JS failed without setting an exception!" with setTimeout

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jesse Ruderman]

Attachment: testcase

JavaScript error: , line 0: Permission denied for <file://> to call method XULElement.QueryInterface

###!!! ASSERTION: JS failed without setting an exception!: 'JS_IsExceptionPending(cx)', file js/src/xpconnect/src/xpcwrappedjsclass.cpp, line 323

Security-sensitive because there seems to be some context or privilege confusion. Did a the file: URL really get its hands on a XULElement?

Comment 1

[Jesse Ruderman]

Attachment: stack trace

Comment 2

[Boris Zbarsky [:bzbarsky]]

We end up with that JS error because nsHTMLLegendElement::Focus calls nsFocusManager::MoveFocus which calls nsFocusManager::DetermineElementToMoveFocus which tries to get the next tabbable content, which ends up calling nsXULElement::IsFocusable on a <xul:textbox>.  This is presumably happening because MoveFocus is used by the legend element to move focus to the next thing in tab order, but there's nothing else in the content document in tab order, so we end up trying to move the focus to chrome.  This is probably buggy; enn, is there a way to restrict the focus move to the content document?

That said, it's odd that trying to call back out through XPConnect there does a security check...

Comment 3

[Neil Deakin]

Should be fairly easy to add. Just implement an extra bitflag passed to MoveFocus. Then when set, DetermineElementToMoveFocus would need to return early here:
http://mxr.mozilla.org/mozilla-central/source/dom/base/nsFocusManager.cpp#2517

(before where it says 'Traverse up to the parent document')

I assume you still want to traverse down into subframes, but this will stop traversal up and to siblings.

Comment 4

[Daniel Veditz [:dveditz]]

There is in fact a security check: does that save us enough or do you think there's still a potential security problem here.

Neil: would such a check interfere with tabbing around a multi-frame document, or just automatic focus moves as in this legend case?

Comment 5

[Neil Deakin]

The intent would be that the flag would only be supplied by the call from nsHTMLLegendElement::Focus.

Comment 6

[Josh Aas]

Neil - any idea when you'll be able to work on this?

Comment 7

[Neil Deakin]

Attachment: Add flag for legend focus

Comment 8

[Josh Aas]

Neil - did you mean to request review here?

Comment 9

[Neil Deakin]

Comment on attachment 591169 [details] [diff] [review]
Add flag for legend focus

Can do, I was waiting to ensure that tests passed which they do.

Comment 10

[Olli Pettay [:smaug][[e-mail]](vacation until July 14)]

Comment on attachment 591169 [details] [diff] [review]
Add flag for legend focus

I guess this is ok.

Comment 11

[Neil Deakin]

http://hg.mozilla.org/mozilla-central/rev/5185e21c55ec

Comment 12

[Daniel Veditz [:dveditz]]

We should be able to do without fixing this on the ESR because the existing security check does prevent bad things happening.