369404 - Assertion failure: !SPROP_HAS_STUB_SETTER(sprop) || (sprop->attrs & JSPROP_GETTER)

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

Debug:
Assertion failure: !SPROP_HAS_STUB_SETTER(sprop) || (sprop->attrs & JSPROP_GETTER), at /Users/jruderman/trunk/mozilla/js/src/jsobj.c:3663

Opt:
Crash with 0 on top --> security-sensitive

This reminds me of bug 367589.

Comment 1

[Jesse Ruderman]

Attachment: testcase

Comment 2

[Brendan Eich [:brendan]]

Attachment: fix

Just missed this case in the js_Native{Get,Set} fixing the other month.

Jesse, why is a null dereference security-sensitive? IIRC that requires either structured exception handling on Windows (which we don't use) or some broken version of Linux (which we should not ship on).

/be

Comment 3

[Brendan Eich [:brendan]]

I don't believe this is exploitable.

/be

Comment 4

[Jesse Ruderman]

I filed it as security-sensitive because I saw 0 as the top item in the stack (as the instruction pointer).  Most bugs that can cause 0 to be the top item in the stack can also cause other numbers to be the top item in the stack.

Comment 5

[Brendan Eich [:brendan]]

It's a guaranteed null pointer dereference -- the assertion is botching due to the null "stub setter" pointer.

Fixed on trunk: js/src/jsobj.c 3.329

/be

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 256280 [details] [diff] [review]
fix

Patch applies cleanly with hunk skew.

/be

Comment 7

[:Gijs (out for now; he/him)]

I think this might have caused bug 371724, is that possible?

Comment 8

[:Gijs (out for now; he/him)]

So yeah, I backed out this patch on current trunk, and that made the testcase on bug 371724 succeed. Marking deps and all that.

Comment 9

[:Gijs (out for now; he/him)]

Er, I backed out locally, that is. Should this bug be reopened?

Really marking deps.

Comment 10

[Brendan Eich [:brendan]]

I took bug 371724 and have a patch in it -- please test it with this bug's patch restored, and update that bug with good news. I will put an all-in-one 1.8* branch patch here when the followup patch for bug 371724 has landed. Thanks,

/be

Comment 11

[Brendan Eich [:brendan]]

Attachment: 1.8* branch patch, including fix for 371724

Comment 12

[Bob Clary [:bc] (inactive)]

/cvsroot/mozilla/js/tests/js1_5/extensions/regress-369404.js,v  <--  regress-369404.js
initial revision: 1.1

Comment 13

[Bob Clary [:bc] (inactive)]

verified fixed 1.9.0 20070320 win/mac*/linux

Comment 14

[Daniel Veditz [:dveditz]]

regressions are usually marked as depends on, not blocking.

Comment 15

[Daniel Veditz [:dveditz]]

Comment on attachment 256476 [details] [diff] [review]
1.8* branch patch, including fix for 371724

approved for 1.8.0.12 and 1.8.1.4, a=dveditz for release-drivers

Comment 16

[Brendan Eich [:brendan]]

Fixed on the

1.8 branch:   js/src/jsobj.c 3.208.2.50
1.8.0 branch: js/src/jsobj.c 3.208.2.12.2.24

/be

Comment 17

[Bob Clary [:bc] (inactive)]

verified fixed linux, windows, mac* 1.8.0, 1.8.1 shell 20070406

Comment 18

[Bob Clary [:bc] (inactive)]

Browser based tests which rely on the delayed ending of the test must call reportCompare before jsTestDriverEnd in order for the test results to be recorded prior to their dumping to stdout.

Checking in regress-369404.js;
/cvsroot/mozilla/js/tests/js1_5/extensions/regress-369404.js,v  <--  regress-369404.js
new revision: 1.4; previous revision: 1.3