Closed Bug 970285 Opened 6 years ago Closed 6 years ago

Crash [@ __memcpy_ssse3_rep] through [@ js::SizedTypeRepresentation::initInstance]

Categories

(Core :: JavaScript Engine, defect, critical)

x86
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla30
Tracking Status
firefox29 --- disabled
firefox30 + fixed
firefox-esr24 --- unaffected
b2g-v1.4 --- disabled

People

(Reporter: decoder, Assigned: nmatsakis)

References

(Blocks 1 open bug)

Details

(Keywords: crash, sec-critical, testcase, Whiteboard: [jsbugmon:update])

Crash Data

Attachments

(2 files, 1 obsolete file)

The following testcase crashes on mozilla-central revision ecf20a2484b6 (run with --fuzzing-safe):


var AA = TypedObject.uint8.array(2147483647).array();
var aa = new AA(-1);
This looks like a bad write to me:

Program received signal SIGSEGV, Segmentation fault.
__memcpy_ssse3_rep () at ../sysdeps/i386/i686/multiarch/memcpy-ssse3-rep.S:1310
#0  __memcpy_ssse3_rep () at ../sysdeps/i386/i686/multiarch/memcpy-ssse3-rep.S:1310
#1  0x080cd376 in js::SizedTypeRepresentation::initInstance (this=0x9500a38, rt=0x9471ff8, mem=0x776ff008 "", length=4294967295) at /usr/include/bits/string3.h:52
#2  0x080db58a in js::TypedObject::createZeroed (cx=0x9488e98, descr=..., length=-1) at js/src/builtin/TypedObject.cpp:2223
#3  0x08103b85 in js::TypedObject::construct (cx=0x9488e98, argc=1, vp=0x9482e10) at js/src/builtin/TypedObject.cpp:2269
#4  0x0858dbd6 in js::CallJSNative (cx=0x9488e98, native=0x8103ac0 <js::TypedObject::construct(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:220
#5  0x0859042b in js::CallJSNativeConstructor (cx=0x9488e98, native=0x8103ac0 <js::TypedObject::construct(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:253
#6  0x0857e213 in js::InvokeConstructor (cx=0x9488e98, args=...) at js/src/vm/Interpreter.cpp:573
#7  0x08575e47 in Interpret (cx=0x9488e98, state=...) at js/src/vm/Interpreter.cpp:2605
eax     0x77800021      2004877345
ebx     0xf7e5cff4      -135933964
ecx     0x7fefef66      2146430822
edx     0xf77fffa0      -142606432
esi     0xf76ff007      -143659001
edi     0x2     2
ebp     0xffffbfc8      4294950856
esp     0xffffbf78      4294950776
eip     0xf7df1186 <__memcpy_ssse3_rep+3510>
=> 0xf7df1186 <__memcpy_ssse3_rep+3510>:        movntdq %xmm6,0x60(%edx)


Marking s-s and sec-critical for now, based on the trace.
Crash Signature: [@ __memcpy_ssse3_rep] through [@ js::SizedTypeRepresentation::initInstance] → [@ __memcpy_ssse3_rep] [@ js::SizedTypeRepresentation::initInstance]
Keywords: sec-critical
Whiteboard: [jsbugmon:update,bisect]
Flags: needinfo?(nmatsakis)
Assignee: nobody → nmatsakis
Flags: needinfo?(nmatsakis)
Goofy. Looks like it'll be an easy fix.
Attached patch Bug970285.diff (obsolete) — Splinter Review
Attachment #8375176 - Flags: review?(shu)
Comment on attachment 8375176 [details] [diff] [review]
Bug970285.diff

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

derp

::: js/src/builtin/TypedObject.cpp
@@ +2257,5 @@
>        case TypeRepresentation::UnsizedArray:
>          // First argument is a length.
> +        if (nextArg >= argc
> +            || !args[nextArg].isInt32()
> +            || args[nextArg].toInt32() < 0)

Style nit: || at end of each line instead of beginning
Attachment #8375176 - Flags: review?(shu) → review+
(In reply to Shu-yu Guo [:shu] from comment #5)
> Style nit: || at end of each line instead of beginning

That's how I prefer it anyway... thought I saw some other code doing it at the beginning.
This doesn't need sec-approval+ to go in since it only affects trunk. Just get it in.
Crash Signature: [@ __memcpy_ssse3_rep] [@ js::SizedTypeRepresentation::initInstance] → [@ __memcpy_ssse3_rep] [@ js::SizedTypeRepresentation::initInstance]
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
JSBugMon: Bisection requested, result:
=== Tinderbox Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20140220102129" and the hash "7a5cbe4dadf8".
The "bad" changeset has the timestamp "20140220102430" and the hash "cc73b1f7a47d".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=7a5cbe4dadf8&tochange=cc73b1f7a47d
Argh, forgot I didn't land this. Do so right now.
Crash Signature: [@ __memcpy_ssse3_rep] [@ js::SizedTypeRepresentation::initInstance] → [@ __memcpy_ssse3_rep] [@ js::SizedTypeRepresentation::initInstance]
Actually I think I fixed this as part of bug 898356, but I'd still like to land the test.
My mistake, not fixed. Fixing now.
Attached patch Bug970285.diffSplinter Review
Rebased. Carrying over r+.
Attachment #8375176 - Attachment is obsolete: true
Attachment #8380792 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/afbf1c788ada
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Status: RESOLVED → VERIFIED
Crash Signature: [@ __memcpy_ssse3_rep] [@ js::SizedTypeRepresentation::initInstance] → [@ __memcpy_ssse3_rep] [@ js::SizedTypeRepresentation::initInstance]
JSBugMon: This bug has been automatically verified fixed.
Group: core-security
You need to log in before you can comment on or make changes to this bug.