Closed
Bug 86028
Opened 23 years ago
Closed 20 years ago
can redefine focus() and other allAccess functions at another domain
Categories
(Core :: Security: CAPS, defect, P2)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jruderman, Assigned: dveditz)
Details
(Keywords: csectype-sop, fixed1.4.3, Whiteboard: [sg:fix])
Attachments
(2 files)
536 bytes,
text/html
|
Details | |
3.90 KB,
patch
|
caillon
:
review+
dveditz
:
superreview+
caillon
:
approval1.4.3+
dveditz
:
approval1.7+
|
Details | Diff | Splinter Review |
I can redefine focus() and several other functions on pages at another domain. I should only be able to call these functions, not redefine them. The redefined function runs with the principal of the attacker, and these functions aren't called with interesting parameters, so this isn't a huge security hole. Here's the line form all.js: pref("capability.policy.default.Window.focus", "allAccess"); I think this should be pref("capability.policy.default.Window.focus.get", "allAccess"); but I couldn't figure out how to test that change.
Reporter | ||
Comment 1•23 years ago
|
||
Updated•23 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P4
Target Milestone: --- → mozilla0.9.3
Comment 2•23 years ago
|
||
Target is now 0.9.4, Priority P2.
Priority: P4 → P2
Target Milestone: mozilla0.9.3 → mozilla0.9.4
Reporter | ||
Comment 3•23 years ago
|
||
Similar to bug 77503, possible to set window.toString for another domain.
Comment 4•23 years ago
|
||
Not critical for 0.9.4, moving to 0.9.5.
Target Milestone: mozilla0.9.4 → mozilla0.9.5
Comment 5•23 years ago
|
||
This appears to be fixed now, but the behavior is a little surprising. When I call x.focus(), where x is a window at another domain, XPConnect calls the security manager regarding a call to Window.focus(). When I attempt to redefine x.focus(), the DOM helper at nsDOMClassInfo calls the AddProperty security check, and that's what gets rejected. I think this is the correct behavior. Jesse, can you verify that this bug is fixed and maybe try some variations?
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Verified on 2001-10-10-branch build on WinNT I get the following error on the Js console when I run the attached test case Error: uncaught exception: Permission denied to set property Window.scriptglobals
Status: RESOLVED → VERIFIED
QA Contact: ckritzer → bsharma
Comment 7•22 years ago
|
||
This bug appears to have re-emerged.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Updated•22 years ago
|
Target Milestone: mozilla0.9.5 → ---
Updated•22 years ago
|
Group: security
Updated•22 years ago
|
Group: security
Assignee | ||
Comment 8•20 years ago
|
||
dang, fell through the cracks!
Assignee: security-bugs → dveditz+bmo
Status: REOPENED → NEW
Flags: blocking1.7?
Whiteboard: [sg:investigate]
Comment 9•20 years ago
|
||
Seems like Jesse's suggestion works as expected, patch coming up.
Comment 10•20 years ago
|
||
Updated•20 years ago
|
Attachment #144693 -
Flags: superreview?(dveditz+bmo)
Attachment #144693 -
Flags: review?(caillon)
Comment 11•20 years ago
|
||
Comment on attachment 144693 [details] [diff] [review] Only allow getting allAccess functions across domains, don't allow setting them. What is http://lxr.mozilla.org/mozilla/source/calendar/sunbird/app/profile/all.js and does that need the same changes? r=caillon once you figure that out.
Attachment #144693 -
Flags: review?(caillon) → review+
Comment 12•20 years ago
|
||
Hmm, messed up. Yeah, that should be updated as well, but I think I'd rather file a bug on Sunbird to not duplicate all.js, since that's a mantenance nightmare, something we really really don't want with files like this. There are better ways. I filed bug 238569 on sunbird to not use a copy of all.js. But for now, I don't think we need to worry about that.
Comment 13•20 years ago
|
||
Fair enough.
Assignee | ||
Comment 14•20 years ago
|
||
What am I missing? If I apply the patch the "demo using blur()" testcase still shows the bug for me.
Whiteboard: [sg:investigate] → [sg:fix]
Comment 15•20 years ago
|
||
Hmm, is your Mozilla build finding other prefs that override what you're setting? Does about:config reflect what you've changed in your all.js pref file?
Comment 16•20 years ago
|
||
Hmm, I guess we don't show these prefs in about:config at all, so that's not very helpful. I double checked that this does indeed fix the problem for me, I tried in a SeaMonkey debug tree, and in a Firefox release tree, problem solved in both cases. I now get an exception when trying to set window.blur across domains (using the attached testcase).
Comment 17•20 years ago
|
||
Those trees were both WinXP builds, tested on Linux (Firefox) as well.
Assignee | ||
Comment 18•20 years ago
|
||
Comment on attachment 144693 [details] [diff] [review] Only allow getting allAccess functions across domains, don't allow setting them. My bad -- misled by the all.js that's still installed under defaults\pref. The active settings from from the [GRE]\greprefs dir. Works just fine sr=dveditz a=dveditz for 1.7
Attachment #144693 -
Flags: superreview?(dveditz+bmo)
Attachment #144693 -
Flags: superreview+
Attachment #144693 -
Flags: approval1.7+
Comment 19•20 years ago
|
||
Fix checked in, closing bug.
Status: NEW → RESOLVED
Closed: 23 years ago → 20 years ago
Resolution: --- → FIXED
Comment 20•20 years ago
|
||
Hard coding of the following pref is carried out in Camino. Isn't this influenced? capability.policy.default.Window.blur capability.policy.default.Window.close capability.policy.default.Window.focus http://lxr.mozilla.org/mozilla/source/camino/src/preferences/PreferenceManager.mm#329 (It may still be in other places.)
Comment 21•20 years ago
|
||
pinkerton-san, Please check, "capability.policy.default.*"
Comment 22•20 years ago
|
||
crot0@infoseek.jp, camino is not setting default prefs, but clearing user set values. Look at the code. By doing so, it is setting everything to the default which is in all.js. If for whatever reason camino does not have this all.js, then the default values would be "sameOrigin" and that is not succeptible to this bug.
Comment 23•20 years ago
|
||
(In reply to comment #16) >Hmm, I guess we don't show these prefs in about:config at all, so that's not >very helpful. That behaviour predates my involvement with about:config so I just carried it forward but perhaps you'd like a pref to show those prefs or something ;-)
Comment 24•20 years ago
|
||
(In reply to comment #23) > (In reply to comment #16) > >Hmm, I guess we don't show these prefs in about:config at all, so that's not > >very helpful. > That behaviour predates my involvement with about:config so I just carried it > forward but perhaps you'd like a pref to show those prefs or something ;-) I'm fine either way. I don't want people to mess with those pefs too much, so there's some value in not showing them to the users of about:config...
Updated•20 years ago
|
Flags: blocking1.7?
Assignee | ||
Comment 25•20 years ago
|
||
Adding Jon Granrose to CC list to help round up QA resources for verification
Updated•20 years ago
|
Attachment #144693 -
Flags: approval1.4.3?
Comment 26•20 years ago
|
||
Comment on attachment 144693 [details] [diff] [review] Only allow getting allAccess functions across domains, don't allow setting them. a=blizzard
Attachment #144693 -
Flags: approval1.4.3? → approval1.4.3+
Updated•20 years ago
|
Keywords: fixed1.4.3
You need to log in
before you can comment on or make changes to this bug.
Description
•