See discussion at: http://groups.google.com/group/mozilla.dev.quality/browse_thread/thread/2708e1fb33220f4b/e7b9be697c7ec449?lnk=st Some functions in nsIDOMWindowUtils can be accessed from unprivileged context, for instance the bookmarklet Vlad describes here: http://blog.vlad1.com/archives/2005/10/28/74/ and the focus method here: http://martijn.martijn.googlepages.com/focus.htm 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.
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
Created attachment 284268 [details] [diff] [review] Adds UniversalXPConnect check to focus
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...
Fix checked in. Thank you Surya!
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
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): http://developer.mozilla.org/en/docs/Writing_MochiTest-based_unit_tests http://developer.mozilla.org/en/docs/Mochitest
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+
Checking in Makefile.in; /cvsroot/mozilla/dom/tests/mochitest/bugs/Makefile.in,v <-- Makefile.in new revision: 1.17; previous revision: 1.16 done RCS file: /cvsroot/mozilla/dom/tests/mochitest/bugs/test_bug397571.html,v done Checking in test_bug397571.html; /cvsroot/mozilla/dom/tests/mochitest/bugs/test_bug397571.html,v <-- test_bug39 7571.html initial revision: 1.1 done 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.