Closed Bug 798011 Opened 11 years ago Closed 11 years ago

mozMatchesSelectorStub crash with Proxy

Categories

(Core :: XPConnect, defect)

Product:
Core
Component:
XPConnect
Platform:
x86_64
macOS
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla18
Tracking Flags:
Tracking Status
firefox17 + verified
firefox18 + verified
firefox-esr10 --- unaffected

People

(Reporter: jruderman, Assigned: bzbarsky)

References

Details

(4 keywords, Whiteboard: [adv-track-main17-])

Crash Data

Attachments

(3 files)

testcase (crashes Firefox when loaded)
232 bytes, text/html
Details
stack
10.71 KB, text/plain
Details
Deal with JS_ValueToString failing.
1.13 KB, patch
gkrizsanits
: review+
akeybl
: approval-mozilla-aurora+
Details | Diff | Splinter Review
Attached file stack — 

    
    

    
  


  

        Attachment #668710 -
        Flags: review?(gkrizsanits)

    
    

    
  
I believe this is a guaranteed null-deref, so not security-sensitive....
Assignee: nobody → bzbarsky
Whiteboard: [need review]

    
    

    
  
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?
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED

    
    

    
  
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+

    
  
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
Group: core-security

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