Last Comment Bug 714276 - nsDOMWindowUtils::NodesFromRect doesn't have a security check
: nsDOMWindowUtils::NodesFromRect doesn't have a security check
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: 10 Branch
: x86_64 Linux
: -- normal (vote)
: mozilla12
Assigned To: :Felipe Gomes (needinfo me!)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-30 07:16 PST by Olli Pettay [:smaug]
Modified: 2012-01-28 19:01 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (5.49 KB, patch)
2012-01-26 19:55 PST, :Felipe Gomes (needinfo me!)
bugs: review+
Details | Diff | Splinter Review

Description Olli Pettay [:smaug] 2011-12-30 07:16:25 PST
I'm not sure whether this is a security bug, but it is strange to let web pages to
use DOMWindowUtils APIs.
Comment 1 Olli Pettay [:smaug] 2011-12-30 07:18:00 PST
If exposing the method to web is ok, it should be added to document.

Someone in #whatwg was asking about such method.
Comment 2 :Felipe Gomes (needinfo me!) 2011-12-30 07:34:00 PST
It doesn't have a check because we decided not to expose it to content at the time. See bug 489127 comment 27 and 37.

Or can webpages now access things from DOMWindowUtils? (If they do I believe this changed recently? I don't think they could when we implemented that feature)

It would be nice to expose that to content imo. IE and Webkit implemented similar APIs after we did ours, though with some slightly different behavior.
Comment 3 Olli Pettay [:smaug] 2011-12-30 07:51:26 PST
Web pages have always had access to DOMWindowUtils.
Comment 4 Daniel Veditz [:dveditz] 2012-01-11 17:26:05 PST
Did this turn out to be a problem or not?
Comment 5 Olli Pettay [:smaug] 2012-01-11 22:28:39 PST
Roc, Felipe? Do we expose anything insecure via NodesFromRect?
Does it have all the necessary cross-iframe checks etc?
Comment 6 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-01-12 01:09:22 PST
We should not expose anything in DOMWindowUtils to Web content because none of it's standard and we don't want to standardize DOMWindowUtils!
Comment 7 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-01-12 01:09:55 PST
So the reason to protect nodesFromRect, or DOMWindowUtils in general, from Web content is not just security but to stop them from relying on stuff there.
Comment 8 :Felipe Gomes (needinfo me!) 2012-01-12 06:14:50 PST
But yeah there's no security risk in this function, the code is very similar to elementFromPoint, with the difference that it returns a list of elements and text nodes instead of just one. It only returns elements from the same document, using the same code as elementFromPoint. So to my best understanding there is nothing insecure here. (can we make the bug public?)

Let's protect this code from being called by webpages anyway due to what Roc mentioned.
Comment 9 Johnny Stenback (:jst, jst@mozilla.com) 2012-01-18 17:41:10 PST
I think we should add a security check here whether there's a security risk here or not. If we don't then sites might start depending on this, and we're not making any promises about any APIs exposed through DOMWindowUtils. Opening this bug cause there doesn't appear to be anything to hide here.
Comment 10 :Felipe Gomes (needinfo me!) 2012-01-26 19:55:07 PST
Created attachment 592035 [details] [diff] [review]
Patch

Trivial change + moving the test for nodesFromRect to mochitest-chrome where it'll have permission to call the function
Comment 11 :Felipe Gomes (needinfo me!) 2012-01-27 16:56:31 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/5095046dc23c
Comment 12 Joe Drew (not getting mail) 2012-01-28 19:01:46 PST
https://hg.mozilla.org/mozilla-central/rev/5095046dc23c

Note You need to log in before you can comment on or make changes to this bug.