Crash [@ js_TraceScopeProperty] with gczeal, setter, watch

VERIFIED FIXED in mozilla1.9.1b3

Status

()

Core
JavaScript Engine
--
critical
VERIFIED FIXED
9 years ago
5 years ago

People

(Reporter: gkw, Assigned: mrbkap)

Tracking

(Blocks: 1 bug, 5 keywords)

Trunk
mozilla1.9.1b3
x86
Mac OS X
crash, fixed1.8.1.21, testcase, verified1.9.0.6, verified1.9.1
Points:
---
Bug Flags:
blocking1.9.1 +
wanted1.9.0.x +
blocking1.8.1.next +
wanted1.8.1.x +
blocking1.8.0.next +
wanted1.8.0.x +
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:nse] null deref, crash signature)

Attachments

(3 attachments)

(Reporter)

Description

9 years ago
Created attachment 337398 [details]
gdb backtraces, normal and full

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?
(Reporter)

Comment 1

9 years ago
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?
(Reporter)

Comment 3

9 years ago
(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.
(Reporter)

Comment 5

9 years ago
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

Updated

9 years ago
Summary: Crash [@ js_TraceScopeProperty] → Crash [@ js_TraceScopeProperty] with gczeal, setter, watch

Updated

9 years ago
Flags: blocking1.9.1?
(Assignee)

Comment 6

9 years ago
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.
(Assignee)

Comment 7

9 years ago
Created attachment 348049 [details] [diff] [review]
Fix
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+

Updated

9 years ago
Attachment #348049 - Flags: review?(igor) → review+
(Assignee)

Comment 9

9 years ago
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+
Comment on attachment 348049 [details] [diff] [review]
Fix

a191=beltzner
(Assignee)

Comment 11

9 years ago
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
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1b3

Comment 13

9 years ago
Created attachment 351392 [details]
js1_5/extensions/regress-454142.js

Updated

9 years ago
Flags: wanted1.9.0.x?
Flags: wanted1.8.1.x?
Flags: in-testsuite+
Flags: in-litmus-

Comment 15

9 years ago
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?]
(Assignee)

Comment 16

9 years ago
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?
(Assignee)

Comment 18

9 years ago
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+

Comment 20

9 years ago
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

Comment 21

9 years ago
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+

Updated

9 years ago
Attachment #348049 - Flags: approval1.8.1.next?
Attachment #348049 - Flags: approval1.8.0.next?

Updated

9 years ago
Flags: blocking1.9.1? → blocking1.9.1+

Comment 22

9 years ago
tests checked in http://hg.mozilla.org/mozilla-central/rev/37c7e3fe4344 and cvs
Bob, can you verify this fix for 1.9.0.6?

Comment 24

9 years ago
v 1.9.0.6
Keywords: fixed1.9.0.6 → verified1.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.

Comment 26

9 years ago
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
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/a2c5f14c8d33
Keywords: verified1.9.1
Crash Signature: [@ js_TraceScopeProperty]
(Reporter)

Updated

5 years ago
Flags: wanted1.9.1?
You need to log in before you can comment on or make changes to this bug.