mozMatchesSelectorStub crash with Proxy

VERIFIED FIXED in Firefox 17

Status

()

Core
XPConnect
--
critical
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: Jesse Ruderman, Assigned: bz)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla18
x86_64
Mac OS X
crash, csectype-dos, csectype-nullptr, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox17+ verified, firefox18+ verified, firefox-esr10 unaffected)

Details

(Whiteboard: [adv-track-main17-], crash signature)

Attachments

(3 attachments)

(Reporter)

Description

5 years ago
Created attachment 668129 [details]
testcase (crashes Firefox when loaded)

bp-5dff7bc0-33af-4d98-a6de-fd44c2121004
(Reporter)

Comment 1

5 years ago
Created attachment 668132 [details]
stack
Created attachment 668710 [details] [diff] [review]
Deal with JS_ValueToString failing.
Attachment #668710 - Flags: review?(gkrizsanits)
I believe this is a guaranteed null-deref, so not security-sensitive....
Assignee: nobody → bzbarsky
Whiteboard: [need review]
Keywords: csec-nullptr
Comment on attachment 668710 [details] [diff] [review]
Deal with JS_ValueToString failing.

Review of attachment 668710 [details] [diff] [review]:
-----------------------------------------------------------------

Just one question. Wouldn't it make sense if an init method that can fail, handled the null case internally? Personally I would put the null check inside nsDependentJSString::init too just in case someone else does the same mistake as I did. Anyway, that being said r+ and thanks for fixing it.
Attachment #668710 - Flags: review?(gkrizsanits) → review+
We could do that, at the cost of an extra null-check for every single existing consumer...
tracking-firefox17: --- → ?
tracking-firefox18: --- → ?
https://hg.mozilla.org/integration/mozilla-inbound/rev/c532f851ec57
Whiteboard: [need review]
Target Milestone: --- → mozilla18
Comment on attachment 668710 [details] [diff] [review]
Deal with JS_ValueToString failing.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 763897
User impact if declined: Null-deref crashes that web pages can trigger
Testing completed (on m-c, etc.): Tested on the attached testcase
Risk to taking this patch (and alternatives if risky): Very low risk.  Just adds
    a missing null-check and exception, instead of crash.
String or UUID changes made by this patch: None.
Attachment #668710 - Flags: approval-mozilla-aurora?

Updated

5 years ago
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
tracking-firefox17: ? → +
tracking-firefox18: ? → +

Comment 8

5 years ago
Comment on attachment 668710 [details] [diff] [review]
Deal with JS_ValueToString failing.

[Triage Comment]
Reproducible crash regression with a very low risk fix - approving for Aurora 17. Please land early Monday to make the next merge.
Attachment #668710 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/21cc2ed4690c
status-firefox17: --- → fixed

Updated

5 years ago
status-firefox-esr10: --- → unaffected
status-firefox18: --- → fixed
Keywords: csec-dos
Keywords: verifyme
Whiteboard: [adv-track-main17-]
Confirmed crash on 2012-10-4
Verified fixed on build 2012-11-13, 17.0b6
Verified fixed on build 2012-11-19, 17.0esr
Verified fixed on build 2012-11-19, 18.0a2 Aurora
Status: RESOLVED → VERIFIED
status-firefox17: fixed → verified
status-firefox18: fixed → verified
Keywords: verifyme
Group: core-security
You need to log in before you can comment on or make changes to this bug.