The following testcase asserts on mozilla-central revision 1d0496e30feb (x86 ARM simulator build, run with --fuzzing-safe --ion-eager): function testFloat32SetElemIC(a) { for (var i = 0; i < a.length; i++) { var r = Math.fround(Number.MAX_VALUE); a[i] = r; assertEq(a[i], r); } } testFloat32SetElemIC(new Array(2048)); testFloat32SetElemIC(new Float32Array(2048));

Crash trace for the release build: Program received signal SIGSEGV, Segmentation fault. JSObject::getGeneric (cx=0x923daa8, obj=(JSObject * const) 0xe0000000 Cannot access memory at address 0xe0000000, receiver=(JSObject * const) 0xe0000000 Cannot access memory at address 0xe0000000, id= $jsid("toSource"), vp=$jsval(-nan(0xfff8200000000))) at js/src/jsobj.h:977 977 js::GenericIdOp op = obj->getOps()->getGeneric; (gdb) bt 16 #0 JSObject::getGeneric (cx=0x923daa8, obj=(JSObject * const) 0xe0000000 Cannot access memory at address 0xe0000000, receiver=(JSObject * const) 0xe0000000 Cannot access memory at address 0xe0000000, id= $jsid("toSource"), vp=$jsval(-nan(0xfff8200000000))) at js/src/jsobj.h:977 #1 0x083b1335 in getProperty (vp=..., name=<optimized out>, receiver=..., obj=..., cx=0x923daa8) at js/src/jsobj.h:1001 #2 js::ValueToSource (cx=0x923daa8, v=$jsval(-nan(0xfffffe0000000))) at js/src/jsstr.cpp:4260 #3 0x0805545e in ToSource (bytes=<synthetic pointer>, vp=..., cx=0x923daa8) at js/src/shell/js.cpp:1587 #4 AssertEq (cx=0x923daa8, argc=2, vp=0xf68fedb8) at js/src/shell/js.cpp:1618 #5 0x082f2593 in js::jit::Simulator::softwareInterrupt (this=0x923d128, instr=0x92ea5f4) at js/src/jit/arm/Simulator-arm.cpp:2156 #6 0x082efa25 in decodeType7 (instr=0x92ea5f4, this=0x923d128) at js/src/jit/arm/Simulator-arm.cpp:3179 #7 js::jit::Simulator::instructionDecode (this=0x923d128, instr=0x92ea5f4) at js/src/jit/arm/Simulator-arm.cpp:4036 #8 0x082f29d0 in execute<false> (this=0x923d128) at js/src/jit/arm/Simulator-arm.cpp:4068 #9 js::jit::Simulator::callInternal (this=0x923d128, entry=0xf64b3648 "\360O-\351\r\200\240\341,\313\001\343#\311", <incomplete sequence \343>) at js/src/jit/arm/Simulator-arm.cpp:4150 #10 0x082f2a92 in js::jit::Simulator::call (this=0x923d128, entry=0xf64b3648 "\360O-\351\r\200\240\341,\313\001\343#\311", <incomplete sequence \343>, argument_count=8) at js/src/jit/arm/Simulator-arm.cpp:4230 #11 0x081e3b32 in EnterIon (data=..., cx=0x923daa8) at js/src/jit/Ion.cpp:2388 #12 js::jit::IonCannon (cx=0x923daa8, state=...) at js/src/jit/Ion.cpp:2469 #13 0x0843aef4 in js::RunScript (cx=0x923daa8, state=...) at js/src/vm/Interpreter.cpp:400 #14 0x0843b9b7 in RunScript (state=..., cx=0x923daa8) at js/src/vm/Interpreter.cpp:388 #15 js::Invoke (cx=0x923daa8, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:493 (More stack frames follow...) (gdb) x /i $pc => 0x80db725 <JSObject::getGeneric(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>)+37>: mov 0x4(%edx),%edx (gdb) info reg edx edx 0xe0000000 -536870912 Crash looks dangerous, marking s-s.

gary: can we run this testcase on your ODROID and see if it's a real ARM problem or if it's a simulator problem?

(In reply to Daniel Veditz [:dveditz] from comment #2) > gary: can we run this testcase on your ODROID and see if it's a real ARM > problem or if it's a simulator problem? It reproduces on hardware here.

Rebase.

Comment on attachment 8415900 [details] [diff] [review] Correct the order of the operands in moveFloat32. [Security approval request comment] How easily could an exploit be constructed based on the patch? It can cause the corruption of a floating point register. Not even sure if it could be exploited, but I don't know. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? There is a test case. Which older supported branches are affected by this flaw? Release, beta, aurora, b2g26, b2g28, b2g30 are affected. b2g18 and esr24 look ok If not all supported branches, which bug introduced the flaw? The bug was introduced in bug 915495. http://hg.mozilla.org/mozilla-central/rev/a43be719866e Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? It is literally a one line fix and is trivial to backport. How likely is this patch to cause regressions; how much testing does it need? It is a low risk of causing regressions. It fixes the test case and has been tested locally with the jit-tests.

We need to get a rating on this as part of getting it approved so we know the severity of this issue. Can this induce a crash at will and how exploitable is it, if so. If this is a sec-high or sec-critical, we'd want the test to be split out and not landed with the fix. The test could land after we ship the fix publicly. That way we don't 0day ourselves with our own test.

Remove the test case.

Split out the test case. Carrying forward the r+ for these.

(In reply to Al Billings [:abillings] from comment #7) > We need to get a rating on this as part of getting it approved so we know > the severity of this issue. Can this induce a crash at will and how > exploitable is it, if so. It can cause a non-canonical floating point value to be passed into C. This causes a assertion failure in the debug builds. It's undefined behaviour and an unknown. How far do we need to explore the potential to exploit this? Is a non-canonical JS value a sufficient exploit risk?

In conversation with Dan Veditz, we think this is a sec-moderate so you can just check into trunk and don't need sec-approval.

Please request Aurora/Beta/b2g28/b2g26 approval on this patch.

Comment on attachment 8415900 [details] [diff] [review] Correct the order of the operands in moveFloat32. [Approval Request Comment] Bug caused by (feature/regressing bug #): The bug was introduced in bug 915495. User impact if declined: Errors in some float computations. Could lead to unexpected JS values being passed into C which could cause a crash. There is an unexplored risk of this being used for an exploit. Testing completed: Locally and on m-c. Risk to taking this patch (and alternatives if risky): Very low. Literally a one line fix. String or UUID changes made by this patch: n/a

Comment on attachment 8415900 [details] [diff] [review] Correct the order of the operands in moveFloat32. This will merge to b2g30 from mozilla-beta.

https://hg.mozilla.org/releases/mozilla-aurora/rev/1389f09bbbc9 https://hg.mozilla.org/releases/mozilla-beta/rev/10e5a05f34b3

Comment on attachment 8415900 [details] [diff] [review] Correct the order of the operands in moveFloat32. Review of attachment 8415900 [details] [diff] [review]: ----------------------------------------------------------------- Taking for sec-moderate

Regarding the query about applying the patch to b2g26 and b2g28. b2g26 does have float32 support so the test is expected to run. The affected function was renamed and the fix would be applied to 'void moveFloat(FloatRegister src, FloatRegister dest)'.

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/10e5a05f34b3 https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/b4b8fe5b4eac https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/7f36f95f0845

JSBugMon: This bug has been automatically verified fixed.

JSBugMon: This bug has been automatically verified fixed on Fx30

JSBugMon: This bug has been automatically verified fixed on Fx31 JSBugMon: This bug has been automatically verified fixed on Fx32