Crash [@ strcmp | nsHTMLMediaElement::Observe] with QI to nsIObserver

VERIFIED FIXED in mozilla2.0b12

Status

()

Core
Audio/Video
--
critical
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: Jesse Ruderman, Assigned: khuey)

Tracking

(Blocks: 1 bug, {crash, testcase})

Trunk
mozilla2.0b12
crash, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 final+)

Details

(Whiteboard: [softblocker], crash signature)

Attachments

(4 attachments)

(Reporter)

Description

7 years ago
Created attachment 484089 [details]
testcase (crashes Firefox when loaded)

2400	nsresult nsHTMLMediaElement::Observe(nsISupports* aSubject,
2401	                                     const char* aTopic, const PRUnichar* aData)
2402	{
2403	  if (strcmp(aTopic, NS_XPCOM_SHUTDOWN_OBSERVER_ID) == 0) {

aTopic is null.
(Reporter)

Comment 1

7 years ago
Breakpad can't handle this crash :(
Assignee: nobody → khuey
(Reporter)

Comment 3

7 years ago
Created attachment 497694 [details]
crash report (mac, debug build of firefox, breakpad)
(Reporter)

Comment 4

7 years ago
Created attachment 497695 [details]
gdb output
(Reporter)

Comment 5

7 years ago
Nominating -- the incomplete stacks for this bug force me to over-ignore when fuzzing, and this won't be fixed by bug 605271 in time for Firefox 4.
blocking2.0: --- → ?
blocking2.0: ? → final+
Whiteboard: [softblocker]
Created attachment 503030 [details] [diff] [review]
Stop script from calling nsHTMLMediaElement::Observe.
Comment on attachment 503030 [details] [diff] [review]
Stop script from calling nsHTMLMediaElement::Observe.

This is ugly, but it gets the job done.  Is getting XPConnect at xpcom-shutdown safe?  I doubt we have test code in the tree to exercise this at xpcom-shutdown.
Attachment #503030 - Flags: review?(jst)
Comment on attachment 503030 [details] [diff] [review]
Stop script from calling nsHTMLMediaElement::Observe.

- In nsHTMLMediaElement::Observe():

+  // Since this is exposed to (malicious) JS, verify that we aren't being
+  // called from script.  We only have to worry about being called directly,
+  // so this is safe.
+  nsAXPCNativeCallContext* ncc = nsnull;
+  nsContentUtils::XPConnect()->GetCurrentNativeCallContext(&ncc);
+  if (ncc)
+    return NS_ERROR_FAILURE;

How about doing what nsImageLoadingContent does in cases like this, which is to call nsContentUtils::IsCallerChrome(), which I believe is shutdown safe. I suspect your version is as well, but I can't say with 100% certainty.

r=jst with that.
Attachment #503030 - Flags: review?(jst) → review+
Made changes suggested by jst and pushed.

http://hg.mozilla.org/mozilla-central/rev/652d012f9dc8
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite+
OS: Mac OS X → All
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b12
Status: RESOLVED → VERIFIED
Crash Signature: [@ strcmp | nsHTMLMediaElement::Observe]
You need to log in before you can comment on or make changes to this bug.