If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Assertion failure: arr->lengthIsWritable() (setter shouldn't be called if property is non-writable), at jsarray.cpp

VERIFIED FIXED in Firefox 27

Status

()

Core
JavaScript Engine
--
critical
VERIFIED FIXED
4 years ago
3 years ago

People

(Reporter: gkw, Assigned: efaust)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla26
x86
Mac OS X
assertion, regression, sec-moderate, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox25 unaffected, firefox26 wontfix, firefox27 fixed, firefox-esr17 unaffected, firefox-esr24 unaffected, b2g18 unaffected)

Details

(Whiteboard: [jsbugmon:update,testComment=3,origRev=1179318fb5aa][adv-main27+])

Attachments

(4 attachments)

(Reporter)

Description

4 years ago
Created attachment 798435 [details]
stack

try {
    x = [];
    y = [];
    f = (function() {
        y.length = 6
    });
    Object.freeze(y);
    f();
    Object.prototype.watch.call(x, "length", (function() {
        return function() {
            f()
        }
    })());
    Array.prototype.pop.call(x)
} catch (e) {}
Array.prototype.push.call(x, "")

asserts js debug threadsafe shell on m-c changeset 1179318fb5aa with --ion-eager --ion-inlining=off at Assertion failure: arr->lengthIsWritable() (setter shouldn't be called if property is non-writable), at jsarray.cpp

My configure flags are:
LD=ld CROSS_COMPILE=1 CXX="clang++ -Qunused-arguments -arch i386" RANLIB=ranlib CC="clang -Qunused-arguments -arch i386" AS=$CC AR=ar STRIP="strip -x -S" HOST_CC="clang -Qunused-arguments" HOST_CXX="clang++ -Qunused-arguments" sh ./configure --target=i386-apple-darwin9.2.0 --enable-macos-target=10.5 --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --enable-threadsafe --with-ccache <some NSPR compile flags, or use --with-system-nspr>

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/c1ccfd8f31bf
user:        Eric Faust
date:        Fri Aug 30 18:50:36 2013 -0700
summary:     Bug 824393 - Part 0: Open SetPropertyIC to cases with uncertain TI. (r=bhackett)

(s-s because the stack is short and strange) Eric, is bug 824393 probably related?
Flags: needinfo?(efaustbmo)
Whiteboard: [jsbugmon:update] → [jsbugmon:]
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
Created attachment 798492 [details]
[crash-signature] Machine-readable crash signature
Easier test that should also reproduce better (m-c rev 1179318fb5aa, options --fuzzing-safe --ion-eager):


x = [ "CNY", "TWD", "invalid" ];
Object.freeze(x).map(function() {
    x.length = 6
})
Whiteboard: [jsbugmon:] → [jsbugmon:update,bisect,testComment=3,origRev=1179318fb5aa]
Whiteboard: [jsbugmon:update,bisect,testComment=3,origRev=1179318fb5aa] → [jsbugmon:update,testComment=3,origRev=1179318fb5aa]
JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/c1ccfd8f31bf
user:        Eric Faust
date:        Fri Aug 30 18:50:36 2013 -0700
summary:     Bug 824393 - Part 0: Open SetPropertyIC to cases with uncertain TI. (r=bhackett)

This iteration took 330.328 seconds to run.
(Assignee)

Comment 5

4 years ago
Created attachment 799015 [details] [diff] [review]
Fix

Yeah, OK. Don't IC unwritable PropertyOp setter shapes. Even though it should be relevant, we mirror the state that is in that flag, so it's the easiest way to check.
Assignee: general → efaustbmo
Status: NEW → ASSIGNED
Attachment #799015 - Flags: review?(jwalden+bmo)
Flags: needinfo?(efaustbmo)
Comment on attachment 799015 [details] [diff] [review]
Fix

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

::: js/src/jit/IonCaches.cpp
@@ +2054,5 @@
>  
>      if (shape->hasSetterValue())
>          return false;
>  
> +    // Despite the vehement claims of Shape.h that writable() is only

I don't actually see these claims, only a commented-out assertion in Shape::writable() that belies them.  :-)
Attachment #799015 - Flags: review?(jwalden+bmo) → review+
How bad is this, security-wise, Eric?
(Assignee)

Comment 8

4 years ago
Best to play it safe here, I think. Jeff and I agree that while the actual assertion that is failing is not a big deal, there could be other PropertyOp setters that are not as carefully written. I don't know what that translates to in terms of flags? moderate is my best guess.

Not hugely bad on the path taken, but with other potential consequences. Suggest we keep it closed until this lands.
Keywords: sec-moderate
Created attachment 800038 [details]
[crash-signature] Machine-readable crash signature
(Assignee)

Comment 10

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/dcd4c1b60d7f
Flags: in-testsuite+
Whiteboard: [jsbugmon:update,testComment=3,origRev=1179318fb5aa] → [jsbugmon:update,testComment=3,origRev=1179318fb5aa,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 680c89c76100).
(Reporter)

Comment 12

4 years ago
This landed on m-c:

http://hg.mozilla.org/mozilla-central/rev/dcd4c1b60d7f
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [jsbugmon:update,testComment=3,origRev=1179318fb5aa,ignore] → [jsbugmon:update,testComment=3,origRev=1179318fb5aa]
(Reporter)

Updated

4 years ago
Target Milestone: --- → mozilla26
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
status-b2g18: --- → unaffected
status-firefox25: --- → unaffected
status-firefox26: --- → wontfix
status-firefox27: --- → fixed
status-firefox-esr17: --- → unaffected
status-firefox-esr24: --- → unaffected
Whiteboard: [jsbugmon:update,testComment=3,origRev=1179318fb5aa] → [jsbugmon:update,testComment=3,origRev=1179318fb5aa][adv-main27+]
Group: core-security
You need to log in before you can comment on or make changes to this bug.