Closed Bug 454142 Opened 11 years ago Closed 11 years ago

Crash [@ js_TraceScopeProperty] with gczeal, setter, watch

Categories

(Core :: JavaScript Engine, defect, critical)

x86
macOS
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla1.9.1b3

People

(Reporter: gkw, Assigned: mrbkap)

References

(Blocks 1 open bug)

Details

(5 keywords, Whiteboard: [sg:nse] null deref)

Crash Data

Attachments

(3 files)

this.watch("x", function(){})
delete x
gczeal(2)
this.__defineSetter__("x", function(){})

blows debug at js_TraceScopeProperty at almost null. Doesn't seem to affect opt.

Occurs with both with and without "-j" option for a js shell compiled from mozilla-central.

Since this concerns gczeal, I'm nominating this as security sensitive.

===

Console output:

$ ./js-dbg-mozTrunk-intelmac 
js> this.watch("x", function(){})
js> delete x
true
js> gczeal(2)
js> this.__defineSetter__("x", function(){})
Bus error

$ ./js-opt-mozTrunk-intelmac 
js> this.watch("x", function(){})
js> delete x
true
js> gczeal(2)
typein:3: ReferenceError: gczeal is not defined
js> this.__defineSetter__("x", function(){})
js>
Flags: wanted1.9.1?
s/nominating/filing

Should have been nominating wanted1.9.1... :-/
(In reply to comment #0)
> Doesn't seem to affect opt.

Is that just because gczeal isn't defined?
(In reply to comment #2)
> (In reply to comment #0)
> > Doesn't seem to affect opt.
> 
> Is that just because gczeal isn't defined?

How should it be defined? (I compiled both debug and opt builds from the same source, mozilla-central, with the same timestamp and no modifications)
It can be defined separately via JS_GC_ZEAL.  I was just asking because of

js> gczeal(2)
typein:3: ReferenceError: gczeal is not defined

in your transcript.
After some investigation, I found that opt builds crash too, but only if I remove the DEBUG ifdef lines above and below this line:

http://mxr.mozilla.org/mozilla-central/source/js/src/jsapi.h#2625

so I guess it can be said that opt builds crash as well...


===

$ ./js
js> this.watch("x", function(){})
js> delete x
true
js> gczeal(2)
js> this.__defineSetter__("x", function(){})
Bus error
Summary: Crash [@ js_TraceScopeProperty] → Crash [@ js_TraceScopeProperty] with gczeal, setter, watch
Flags: blocking1.9.1?
This is a non-exploitable null pointer dereference. The sprop-marking code added in bug 381374 assumed that we'd be overwriting if a watchpoint already existed. This is clearly not the case.
Attached patch FixSplinter Review
Assignee: general → mrbkap
Status: NEW → ASSIGNED
Attachment #348049 - Flags: review?(brendan)
Comment on attachment 348049 [details] [diff] [review]
Fix

Get Igor on this case too.

/be
Attachment #348049 - Flags: review?(igor)
Attachment #348049 - Flags: review?(brendan)
Attachment #348049 - Flags: review+
Attachment #348049 - Flags: review?(igor) → review+
Comment on attachment 348049 [details] [diff] [review]
Fix

Not sure if we want this on the 1.9.0 branch, but it's a stability fix that's very small and very safe so it seems worth it.
Attachment #348049 - Flags: approval1.9.1?
Attachment #348049 - Flags: approval1.9.0.6?
Attachment #348049 - Flags: approval1.9.1? → approval1.9.1+
Checkin message: Bug 454142 - Don't assume that we're overwriting since watchpoints are not tied to the underlying sprop. r=brendan/igor a=beltzner
I didn't see comment 11 until now.  Checkin message just has the bug number and reviewers in it.

http://hg.mozilla.org/mozilla-central/rev/a2c5f14c8d33
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b3
Flags: wanted1.9.0.x?
Flags: wanted1.8.1.x?
Flags: in-testsuite+
Flags: in-litmus-
verified fixed mozilla-central, but not tracemonkey.
Status: RESOLVED → VERIFIED
Flags: wanted1.9.0.x?
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.6+
Whiteboard: [sg:critical?]
Dan, this isn't critical -- it's a null pointer deref.
Whiteboard: [sg:critical?] → [sg:nse] null deref
so we can unhide the bug, right?
Flags: blocking1.9.0.6+ → blocking1.9.0.6?
Yep.
Group: core-security
Flags: blocking1.9.0.6?
Comment on attachment 348049 [details] [diff] [review]
Fix

Approved for 1.9.0.6, a=dveditz for release-drivers.
Attachment #348049 - Flags: approval1.9.0.6? → approval1.9.0.6+
fixed for 1.9.0.6

Checking in js/src/jsscope.c;
/cvsroot/mozilla/js/src/jsscope.c,v  <--  jsscope.c
new revision: 3.89; previous revision: 3.88
done
Keywords: fixed1.9.0.6
couldn't get gczeal enabled in 1.8.1, so couldnt check the testcase. But code patched here exists on 1.8 branches and seems to make sense. setting flags ...
Flags: wanted1.8.1.x?
Flags: wanted1.8.1.x+
Flags: wanted1.8.0.x+
Flags: blocking1.8.1.next?
Flags: blocking1.8.0.next+
Attachment #348049 - Flags: approval1.8.1.next?
Attachment #348049 - Flags: approval1.8.0.next?
Flags: blocking1.9.1? → blocking1.9.1+
Bob, can you verify this fix for 1.9.0.6?
v 1.9.0.6
Flags: blocking1.8.1.next? → blocking1.8.1.next+
Attachment #348049 - Flags: approval1.8.1.next? → approval1.8.1.next+
Comment on attachment 348049 [details] [diff] [review]
Fix

Approved for 1.8.1.21, a=dveditz for release-drivers.
fixed for 1.8.1.21

/cvsroot/mozilla/js/src/jsscope.c,v  <--  jsscope.c
new revision: 3.45.20.7; previous revision: 3.45.20.6
Keywords: fixed1.8.1.21
Crash Signature: [@ js_TraceScopeProperty]
Flags: wanted1.9.1?
You need to log in before you can comment on or make changes to this bug.