Some functions in nsIDOMWindowUtils can be used from unprivileged context




11 years ago
11 years ago


(Reporter: Martijn Wargers (zombie), Assigned: Surya Ismail)


Windows XP
Bug Flags:
blocking1.9 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)




(2 attachments)



11 years ago
See discussion at:

Some functions in nsIDOMWindowUtils can be accessed from unprivileged context, for instance the bookmarklet Vlad describes here:

and the focus method here:

Boris doesn't think that's a good idea.
We need to either close this off from websites (all of nsIDOMWindowUtils) or security-audit the things hanging off it.  People have been adding stuff while assuming that it's not accessible from content, as far as I know.

In particular, I see no reason that sites should be able to force redraw without going back to the event loop.  It encourages scripts that hog the CPU.
Flags: blocking1.9?
The Redraw() stuff was added to WindowUtils on jst's suggestion with the explicit goal that it be accessible from content (e.g. see the bookmarklet).  It's not really useful for scripts in normal use, so worst case it could be used for a script to spike the CPU... but they can already do that in other ways, so I don't think this is any worse.
I see.  Then perhaps we need to more clearly document that some things on nsIWindowUtils are content-accessible.  I assume we've audited the existing things and made sure they're safe?
The only thing that looks potentially scary is the focus function given how rickety our focus code is. So we should add UniversalXPConnect checks to that function and a big-ass warning at the top of the interface file. Also add documentation to the other functions that do UniversalXPConnect checks that they do so.
Assignee: nobody → jst
Flags: blocking1.9? → blocking1.9+
Surya, this would be a great bug to start with. All that's needed here is a change to add a security check in the nsDOMWindowUtils::Focus() method (just like what's done in nsDOMWindowUtils::SendMouseEvent()) and some comment changes to make the method descriptions in the interface nsIDOMWindowUtils match the implementation wrt whether the implementation checks if the caller is chrome or not (has UniversalXPConnect enabled). Let me know if you have any questions etc.
Assignee: jst → suryaismail


11 years ago

Comment 6

11 years ago
Created attachment 284268 [details] [diff] [review]
Adds UniversalXPConnect check to focus
Attachment #284268 - Flags: review?(jst)
Comment on attachment 284268 [details] [diff] [review]
Adds UniversalXPConnect check to focus 

Looks good! r+sr+a=jst

I'll land this for you in a bit...
Attachment #284268 - Flags: superreview+
Attachment #284268 - Flags: review?(jst)
Attachment #284268 - Flags: review+
Attachment #284268 - Flags: approval1.9+
Fix checked in. Thank you Surya!
Last Resolved: 11 years ago
Resolution: --- → FIXED

Comment 9

11 years ago
Surya, could you write a Mochitest that double-checks that these methods/properties all throw/don't throw correctly when called from unprivileged content?  See the following URLs for details, or look at examples in the tree (e.g. content/html/document/test):
Flags: in-testsuite?

Comment 10

11 years ago
Created attachment 294166 [details] [diff] [review]
MochiTest for events

Sorry for delay


11 years ago
Attachment #294166 - Flags: review?(jwalden+bmo)

Comment 11

11 years ago
Comment on attachment 294166 [details] [diff] [review]
MochiTest for events

I tested this and it looks ok to me.
I'll check this in shortly when the tree gets green.

I guess you could add garbageCollect() and redraw() to the test if you like.
Attachment #294166 - Flags: review?(jwalden+bmo) → review+

Comment 12

11 years ago
Checking in;
/cvsroot/mozilla/dom/tests/mochitest/bugs/,v  <--
new revision: 1.17; previous revision: 1.16
RCS file: /cvsroot/mozilla/dom/tests/mochitest/bugs/test_bug397571.html,v
Checking in test_bug397571.html;
/cvsroot/mozilla/dom/tests/mochitest/bugs/test_bug397571.html,v  <--  test_bug39
initial revision: 1.1

Checked in the Mochitest, thanks!
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.