Last Comment Bug 783867 - (CVE-2012-3989) ASSERTION: This only works on nsISupports classes! and segfault
: ASSERTION: This only works on nsISupports classes! and segfault
: csectype-wildptr, regression, sec-critical
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla17
Assigned To: Boris Zbarsky [:bz]
Depends on:
Blocks: ParisBindings 740069
  Show dependency treegraph
Reported: 2012-08-19 03:03 PDT by :Ms2ger (⌚ UTC+1/+2)
Modified: 2014-07-24 13:43 PDT (History)
12 users (show)
dveditz: sec‑bounty+
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Don't blindly assume IsDOMClass objects are nsISupports. (1.49 KB, patch)
2012-08-22 11:25 PDT, Boris Zbarsky [:bz]
peterv: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description :Ms2ger (⌚ UTC+1/+2) 2012-08-19 03:03:47 PDT

new Worker("") instanceof Components.interfaces.nsIArray

in a mochitest will hit

             "This only works on nsISupports classes!");

at <>. We then unwrap the object to nsISupports and try to call QueryInterface on it, and segfault in nsWrapperCache::WrapObject.
Comment 1 David Bolter [:davidb] 2012-08-22 10:11:52 PDT
How dangerous is this?
Comment 2 Boris Zbarsky [:bz] 2012-08-22 10:20:26 PDT
This is reasonably bad.  It's also a regression from the original landing in bug 740069.

Probably too late to fix this on 15 now, but we should fix elsewhere.  Also, not check in a test for this until after we ship the fix.  :(
Comment 3 Boris Zbarsky [:bz] 2012-08-22 11:25:59 PDT
Created attachment 654288 [details] [diff] [review]
Don't blindly assume IsDOMClass objects are nsISupports.
Comment 4 Boris Zbarsky [:bz] 2012-08-22 12:20:24 PDT

No test landed yet; we should do that once we reopen it.
Comment 5 Boris Zbarsky [:bz] 2012-08-22 12:21:35 PDT
Comment on attachment 654288 [details] [diff] [review]
Don't blindly assume IsDOMClass objects are nsISupports.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 740069
User impact if declined: Possibly-exploitable crash
Testing completed (on m-c, etc.): Tested on testcase in this bug
Risk to taking this patch (and alternatives if risky): Very safe: just makes us
  return false in cases where it should be returned, instead of crashing
String or UUID changes made by this patch:
Comment 6 Ed Morley [:emorley] 2012-08-23 03:57:19 PDT
Comment 7 Ed Morley [:emorley] 2012-08-23 03:58:35 PDT
Apologies, had two security bugs open to mark, and pasted other s-g bug's cset in this one.

The correct changeset is:
Comment 9 Al Billings [:abillings] 2012-08-27 16:45:43 PDT
Can someone suggest a sec rating for this issue?

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