Last Comment Bug 705651 - (CVE-2012-0446) Frame scripts that access untrusted objects are exploitable
: Frame scripts that access untrusted objects are exploitable
: verified-beta
Product: Core
Classification: Components
Component: Security (show other bugs)
: unspecified
: x86 Windows XP
: -- normal (vote)
: mozilla11
Assigned To: Olli Pettay [:smaug]
: David Keeler [:keeler] (use needinfo?)
Depends on:
Blocks: 698772
  Show dependency treegraph
Reported: 2011-11-28 06:26 PST by moz_bug_r_a4
Modified: 2012-08-15 08:31 PDT (History)
11 users (show)
dveditz: sec‑review+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

screenshot of testcase 3 (31.65 KB, image/jpeg)
2011-11-28 06:31 PST, moz_bug_r_a4
no flags Details
patch (8.49 KB, patch)
2011-11-29 02:50 PST, Olli Pettay [:smaug]
mrbkap: review+
Details | Diff | Splinter Review
up-to-date (8.37 KB, patch)
2011-11-30 07:50 PST, Olli Pettay [:smaug]
no flags Details | Diff | Splinter Review
for aurora (8.49 KB, patch)
2011-12-01 03:10 PST, Olli Pettay [:smaug]
christian: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description moz_bug_r_a4 2011-11-28 06:26:31 PST
Frame scripts run on the special JS context for which we call SetSecurityManagerForJSContext with flags=0, thus if a frame script calls into an untrusted function, XPConnect does not do proper security checks.
Comment 4 moz_bug_r_a4 2011-11-28 06:31:46 PST
Created attachment 577241 [details]
screenshot of testcase 3
Comment 5 Olli Pettay [:smaug] 2011-11-28 09:31:11 PST
Nice :(

mrbkap, what kinds of flags can be passed to SetSecurityManagerForJSContext?
There is no documentation in nsIXPConnect nor in nsXPConnect.cpp
Comment 6 Olli Pettay [:smaug] 2011-11-28 09:34:54 PST
I'm pretty sure 0 is passed because it is (or was) passed also here
Comment 7 Olli Pettay [:smaug] 2011-11-28 09:42:32 PST
Ahaa, some documentation is here
Should I pass HOOK_ALL ?
Comment 8 Blake Kaplan (:mrbkap) 2011-11-29 01:23:51 PST
Hey Olli. I don't think you actually need to call SetSecurityManagerForJSContext at all. That function is useful if a given context needs to use a different security manager from the rest of the runtime. However, in most cases, we can just use the one that's installed on XPConnect itself. On the other hand, using HOOK_ALL would also fix this bug.
Comment 9 Olli Pettay [:smaug] 2011-11-29 02:50:45 PST
Created attachment 577535 [details] [diff] [review]

Create a helper method.
Comment 10 Blake Kaplan (:mrbkap) 2011-11-30 06:04:56 PST
Comment on attachment 577535 [details] [diff] [review]

Review of attachment 577535 [details] [diff] [review]:

I mentioned on IRC that the reason that web workers got away with this (before the recent rewrite) is that they truly used object capabilities to protect themselves. Furthermore, they couldn't use the script security manager off the main thread, and therefore wanted to *prevent* calls to the SSM. Because there was nothing around to protect, this was safe.

::: content/base/src/nsFrameMessageManager.cpp
@@ +840,5 @@
> +
> +  JSAutoRequest ar(cx);
> +  nsIXPConnect* xpc = nsContentUtils::XPConnect();
> +  const PRUint32 flags = nsIXPConnect::INIT_JS_STANDARD_CLASSES |
> +                         /*nsIXPConnect::OMIT_COMPONENTS_OBJECT ?  |*/

I doubt you want this flag. It's only useful if you want to cut the scripts you're going to be running off from XPCOM entirely.
Comment 11 Olli Pettay [:smaug] 2011-11-30 07:45:08 PST
Yeah, that flag has been there since the beginning when it wasn't sure whether we want to
expose components to frame scripts or let them do only simpler things.
I'll remove that.
Comment 12 Olli Pettay [:smaug] 2011-11-30 07:50:30 PST
Created attachment 577960 [details] [diff] [review]
Comment 13 Olli Pettay [:smaug] 2011-11-30 08:33:20 PST
Comment 14 Olli Pettay [:smaug] 2011-11-30 08:34:55 PST
I'll prepare patches for aurora and beta.
Comment 15 Olli Pettay [:smaug] 2011-12-01 03:10:50 PST
Created attachment 578220 [details] [diff] [review]
for aurora

The only difference to trunk is that trunk doesn't have
Comment 16 Olli Pettay [:smaug] 2011-12-01 03:21:49 PST
In beta there is also JS_SetScriptStackQuota and  JSOPTION_ANONFUNFIX |
Comment 17 Alex Keybl [:akeybl] 2011-12-06 12:36:31 PST
Now tracking for FF9/10, and added the sec-review-needed keyword. Please address possible risk for taking on aurora/beta.
Comment 18 Daniel Veditz [:dveditz] 2011-12-13 18:25:00 PST
There is little risk here, we should take this for aurora (fx10)
Comment 19 Daniel Veditz [:dveditz] 2011-12-13 18:30:47 PST
little risk from the patch, I mean! It's a bad security hole that's probably exploitable through several addons and definitely exploitable through an XSS on AMO.
Comment 21 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2012-03-06 15:28:36 PST
Verified fixed in Firefox 11.0b6.
Comment 22 Jason Smith [:jsmith] 2012-03-13 17:05:51 PDT
Verified fixed in Firefox Aurora.
Comment 23 Jason Smith [:jsmith] 2012-03-13 17:07:52 PDT
Verified fixed in Firefox Nightly.

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