Firefox crash by checking x instanceof Components.Exception when x is not an object

This is crashing somewhere in JS land. Odd. Jan, any idea what's up here? Sample crashreports: 53: https://crash-stats.mozilla.com/report/index/cb0e4ce9-1b8b-4792-abf3-f59d52170325 55: https://crash-stats.mozilla.com/report/index/70eb054a-6f02-4643-a2ed-748672170325 (latter looks like it has a more useful stack) STR: 1. eval in browser toolbox: null instanceof Components.Exception

We end up in nsXPCComponents_Exception::HasInstance. There we pass v.toObjectOrNull() to UNWRAP_OBJECT: http://searchfox.org/mozilla-central/rev/7419b368156a6efa24777b21b0e5706be89a9c2f/js/xpconnect/src/XPCComponents.cpp#1713 Then in UnwrapObject we call GetDOMClass and assume |obj| is nullptr. http://searchfox.org/mozilla-central/rev/7419b368156a6efa24777b21b0e5706be89a9c2f/dom/bindings/BindingUtils.h#203 Hiding this as I really don't like that unchecked toObjectOrNull() call.

bz, any thoughts on this? Is this code available to content?

(In reply to Jan de Mooij [:jandem] from comment #2) > Then in UnwrapObject we call GetDOMClass and assume |obj| is nullptr. I meant non-nullptr, obviously.

Components.Exception is not available to content, I think, so I'm not sure this is reachable from public web code, which might affect security-severity?

This should not be available to content, no. This is a clear bug that was introduced in <https://hg.mozilla.org/mozilla-central/rev/16cf9da0d24282153c9ea53e49439dcb3aac194f>: The UNWRAP_OBJECT things should only happen if isObject().

MozReview-Commit-ID: 8lwCoilRz49

I'll mark this sec-other because it isn't clear this is really a problem, but we'd like to leave it hidden in case it is.

(In reply to Boris Zbarsky [:bz] (still a bit busy) (if a patch has no decent message, automatic r-) from comment #7) > Created attachment 8851622 [details] [diff] [review] I assume the CanvasRenderingContext2D.cpp fontMetrics stuff in the patch is part of some other fix?

Er... yes, it is!

Comment on attachment 8851697 [details] [diff] [review] Need to check that the value is an object before we try to UNWRAP_OBJECT it to check whether it's an Exception [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: Simple crash fix for a possibly-exploitable crash, if it can be triggered. User impact if declined: Crashes if extensions do the wrong thing. Fix Landed on Version: 55 Risk to taking this patch (and alternatives if risky): Very low risk. String or UUID changes made by this patch: None. See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info. Approval Request Comment [Feature/Bug causing the regression]: Bug 911258 [User impact if declined]: Possibly-exploitable crash, if it can be triggered. [Is this code covered by automated tests?]: No [Has the fix been verified in Nightly?]: Yes, on my local machine. [Needs manual test from QE? If yes, steps to reproduce]: See comment 1 [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: Just adds a missing type of value check. [String changes made/needed]: None.

Comment on attachment 8851697 [details] [diff] [review] Need to check that the value is an object before we try to UNWRAP_OBJECT it to check whether it's an Exception crash fix for aurora and beta

https://hg.mozilla.org/releases/mozilla-aurora/rev/3e5bbabd5279 https://hg.mozilla.org/releases/mozilla-beta/rev/4409827e9906

Flagging this for manual testing, str available in Comment 1.

Comment on attachment 8851697 [details] [diff] [review] Need to check that the value is an object before we try to UNWRAP_OBJECT it to check whether it's an Exception let's take this in esr52 as well now

Reproduced the initial crash using 53 beta 5, verified that the crash no longer happens using Firefox 53 beta 9, latest Developer Edition 54.0a2 and latest Nightly 55.0a1 on macOS 10.12.4, Windows 10 64bit and Ubuntu 16.04 32bit.

Bogdan, could you please take a look at this on 52.1esr as well?

(In reply to Andrei Vaida, QA [:avaida] – please ni? me from comment #20) > Bogdan, could you please take a look at this on 52.1esr as well? I can also confirm that Fx 52.1.0esr-build3 does not crash across platforms (macOS 10.12.4, Windows 10 64bit and Ubuntu 16.04 32bit).

Comment on attachment 8851697 [details] [diff] [review] Need to check that the value is an object before we try to UNWRAP_OBJECT it to check whether it's an Exception esr45 is gone.