Closed Bug 1001547 Opened 6 years ago Closed 6 years ago

Assertion failure: index < tarray.length(), at vm/TypedArrayObject.cpp

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla31
Tracking Status
firefox29 --- unaffected
firefox30 --- unaffected
firefox31 --- fixed
firefox-esr24 --- unaffected
b2g-v1.2 --- unaffected
b2g-v1.3 --- unaffected
b2g-v1.4 --- unaffected
b2g-v2.0 --- fixed

People

(Reporter: gkw, Assigned: Waldo)

References

Details

(4 keywords, Whiteboard: [jsbugmon:update])

Attachments

(3 files)

Attached file stack
x = ArrayBuffer(64)
valueOf = function() {
    neuter(x)
};
Uint32Array(x)[4] = this

asserts js debug shell on m-c changeset 5ecd532a167e without any CLI arguments at Assertion failure: index < tarray.length(), at vm/TypedArrayObject.cpp

My configure flags are:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --disable-tests --with-ccache --enable-threadsafe <other NSPR options>

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/58ccbf57d72a
user:        Jeff Walden
date:        Thu Mar 20 16:38:12 2014 -0700
summary:     Bug 985733 - Make assignments into typed arrays use ToNumber-style semantics.  r=sfink, r=jandem

Setting s-s by default due to this involving TypedArrays, but pending further analysis I won't be setting a rating just yet.

Waldo, is bug 985733 a likely regressor?
Flags: needinfo?(jwalden+bmo)
That's...odd.  I remember being specifically aware of this potential when fixing this, and I thought sufficiently careful, yet looking at the code now, I don't actually see a place that enforces in-boundsness here.  Blah.

Fix after I get to the office, and maybe after lunch, depending.
Flags: needinfo?(jwalden+bmo)
Assignee: nobody → jwalden+bmo
Status: NEW → ASSIGNED
Comment on attachment 8412843 [details] [diff] [review]
Bounds-checking for dummies.  NOT REVIEWED YET

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

::: js/src/vm/TypedArrayObject.cpp
@@ +1978,4 @@
>  void
>  TypedArrayObject::setElement(TypedArrayObject &obj, uint32_t index, double d)
>  {
> +    if (index >= obj.length())

Please comment why this can't set an error, as you did in the test. Or rather, why it's (more or less) ok that it doesn't.
Attachment #8412843 - Flags: review?(sphink) → review+
Attachment #8412842 - Flags: review?(sphink) → review+
sec-critical, but bug 985733 landed this cycle, so let's get 'er done now with minimal fuss.

https://hg.mozilla.org/integration/mozilla-inbound/rev/d1b899e683c2

I held off on the test for the moment (and would like this bug to remain hidden for now) because the neuter() usage has me somewhat paranoid that it might give malicious readers too many ideas, while we are still in a state of neutering-unsafety (bug 991981, bug 999651, many others still in flight).  :-\  Once that's all wrapped up I'll go back and land the test (probably on any branches this bugfix needs to make its way into, as well -- only aurora if I can move fast, and I dearly hope I can), and we can open up this bug.  But please -- no sooner.
Keywords: sec-critical
OS: Mac OS X → All
Hardware: x86_64 → All
Target Milestone: --- → mozilla31
https://hg.mozilla.org/mozilla-central/rev/d1b899e683c2
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Flags: in-testsuite? → in-testsuite+
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.