Assertion failure: end <= tarray->length(), at vm/TypedArrayObject.cpp or Assertion failure: begin <= tarray->length(), at vm/TypedArrayObject.cpp

VERIFIED FIXED in Firefox 30

Status

()

defect
--
critical
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: gkw, Assigned: Waldo)

Tracking

(Blocks 1 bug, 4 keywords)

Trunk
mozilla32
x86_64
macOS
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox29 wontfix, firefox30 fixed, firefox31 fixed, firefox32 fixed, firefox-esr2430+ fixed, b2g-v1.2 fixed, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed, b2g-v2.0 fixed, seamonkey2.26 wontfix)

Details

(Whiteboard: [jsbugmon:update,testComment=14,origRev=e9cb7cf27ce7][adv-main30+][adv-esr24.6+])

Attachments

(1 attachment)

Reporter

Description

5 years ago
Posted file stack
x = ArrayBuffer(4)
Uint8Array(x).subarray({
    valueOf: function() {
        neuter(x)
    }
})

asserts js debug shell on m-i changeset 52f43e3f552f without any CLI arguments at Assertion failure: end <= 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-inbound/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/6d4ff510c117
user:        Luke Wagner
date:        Thu Oct 24 08:59:59 2013 -0500
summary:     Bug 929786 - Add shell function to neutering (r=sfink)

s-s because this involves typed arrays and neuter.

I don't think it's bug 929786, instead this might be an improvement to jsfunfuzz post-pwn2own that caused this to show up. If anyone comes up with a testcase that does not involve the neuter function, we can possibly bisect further back if necessary.

Setting needinfo? from Waldo since he (and Steve) took a look at most of the typed array issues that came up during pwn2own.
Flags: needinfo?(jwalden+bmo)
In place of neuter(x), use serialize(x, [x]). That'll go back to the introduction of neutering.
Flags: needinfo?(jwalden+bmo)
Oh, ugh. This is exactly what Waldo warned about in bug 982974 comment 47. This fuzz case is the exact same assert as in bug 982974, but is expected to occur. It would no longer cause a problem in an opt build, but will assert in a debug build. (See Waldo's getBuildConfiguration().debug test in that comment.)

Comment 3

5 years ago
With serialize(x, [x]) replacing neuter(x), I get an equally unhelpful bisect result:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/7cc3e16e4af1
user:        Steve Fink
date:        Tue Oct 15 23:47:26 2013 -0700
summary:     Bug 861925 - Add an optional parameter to the shell serialize() function for specifying Transferables, r=jorendorff
Reporter

Comment 4

5 years ago
Yes, I got that bisect result too.

So Steve and I spoke, and it was concluded that we'd ignore this assert (and its "begin <= ..." variant) until the proper comprehensive ArrayBuffer fix lands, unless any of our testcases crash opt builds, then it'll be a different situation altogether.
Reporter

Updated

5 years ago
Summary: Assertion failure: end <= tarray->length(), at vm/TypedArrayObject.cpp → Assertion failure: end <= tarray->length(), at vm/TypedArrayObject.cpp or Assertion failure: begin <= tarray->length(), at vm/TypedArrayObject.cpp
"ignore" and yet this bug is still open? Is that to re-test later, or can we close the bug. Is there a sec-something problem here or do you just want it to stay hidden because of the reference to the other TypedObjectArray issues?  (If it's just hanging around for a later re-test please mark sec-other.)
Flags: needinfo?(gary)
Assignee

Comment 6

5 years ago
It'll go away when the full fix for the underlying issue is completed.  And maybe a delay past that for the current workaround/hackaround to be removed.  I would rather it stay hidden because of the reference to the old issue, yes.
Reporter

Comment 7

5 years ago
Setting sec-other as per comment 5 and comment 6.
Flags: needinfo?(gary)
Keywords: sec-other
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision fcf19894d9f3).
Reporter

Comment 9

5 years ago
Waldo, is this possibly fixed by bug 1001547?
Flags: needinfo?(jwalden+bmo)
Assignee

Comment 10

5 years ago
More likely bug 999755 requires this test be updated to continue to trigger a crash.  (You probably want to update your fuzzers accordingly for that change, because every existing call to neuter will now throw rather than doing what it was once expected to do.)

Bug 991981, and specifically attachment 8409941 [details] [diff] [review], are likely the *actual* eventual fix.
Flags: needinfo?(jwalden+bmo)
Whiteboard: [jsbugmon:update,ignore] → [jsbugmon:bisectfix]
Whiteboard: [jsbugmon:bisectfix] → [jsbugmon:]
JSBugMon: Fix Bisection requested, result:
=== Tinderbox Build Bisection Results by autoBisect ===

The "bad" changeset has the timestamp "20140425114406" and the hash "83ea7136632f".
The "good" changeset has the timestamp "20140425120234" and the hash "aa534ca9cea5".

Likely fix window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=83ea7136632f&tochange=aa534ca9cea5
"Fix" indeed points to bug 999755. Waldo, how would we properly rewrite the test in comment 0 to still reproduce here?
Flags: needinfo?(jwalden+bmo)
(In reply to Christian Holler (:decoder) from comment #12)
> "Fix" indeed points to bug 999755. Waldo, how would we properly rewrite the
> test in comment 0 to still reproduce here?

Change the call to neuter(x) to be neuter(x, "change-data").
Flags: needinfo?(jwalden+bmo)
Thanks Waldo :) Let's try this:


x = ArrayBuffer(4)
Uint8Array(x).subarray({
    valueOf: function() {
        neuter(x, "change-data")
    }
})
Whiteboard: [jsbugmon:] → [jsbugmon:update,testComment=14,origRev=e9cb7cf27ce7]
Group: core-security
Whiteboard: [jsbugmon:update,testComment=14,origRev=e9cb7cf27ce7] → [jsbugmon:update,testComment=14,origRev=e9cb7cf27ce7,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 5add3261493b).
Reporter

Comment 16

5 years ago
Bug 991981 likely fixed this. See comment 10.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [jsbugmon:update,testComment=14,origRev=e9cb7cf27ce7,ignore] → [jsbugmon:update,testComment=14,origRev=e9cb7cf27ce7]
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
Assignee: nobody → jwalden+bmo
Depends on: 991981
Target Milestone: --- → mozilla32
Whiteboard: [jsbugmon:update,testComment=14,origRev=e9cb7cf27ce7] → [jsbugmon:update,testComment=14,origRev=e9cb7cf27ce7][adv-main30+][adv-esr24.6+]
Group: javascript-core-security
trying to apply this to SeaMonkey 2.26.1 (Gecko 29) resulted in patch conflicts, and due to the nature of this patchset it seems like I won't be able to take it.
Group: core-security
You need to log in before you can comment on or make changes to this bug.