Security: CSA_ASSERT failed: IsNumberArrayIndex(k()) [../../v8/src/builtins/builtins-array-gen.cc:592]
Reported by
chinaxia...@gmail.com,
May 28 2018
|
||||||||||||
Issue description
PoC:
function a() {
}
function b() {
try { Array.prototype.findIndex.call(c, c);} catch(e) { }
}
function c(arg1) {
Array.prototype.filter.call('aaaaaaaaaa', d);
o = {}
o.length = Infinity;
Array.prototype.reduceRight.call(o, a);
}
function d() {
o = new Array(10);
Array.prototype.find.call(o, b);
}
var o = Array( a);
o.every(d);
The PoC crash in debug mode.
Debug mode log:
abort: CSA_ASSERT failed: IsNumberArrayIndex(k()) [../../v8/src/builtins/builtins-array-gen.cc:592]
2
3
4 ==== JS stack trace =========================================
5
6 0: ExitFrame [pc: 0x7ee097035664]
7 1: StubFrame [pc: 0x7ee096f8d524]
8 Security context: 0x7ee70f9a49c1 <JSObject>#0#
9 2: reduceRight [0x7ee70f986fe9](this=0x7ee7c4ac8849 <Object map = 0x7ee78ba0dbf9>#1#,0x7ee70f9aaaa1 <JSFunction a (sfi = 0x7ee70f9aa659)>#2#)
10 3: c [0x7ee70f9aab71] [/Users/xiaozhouzhou/Desktop/poc.js:12] [bytecode=0x7ee70f9ab259 offset=88](this=0x7ee7c4a83721 <JSGlobal Object>#3#,arg1=0x7ee7ad1826f1 <undefined>)
11 4: arguments adaptor frame: 3->1
12 5: StubFrame [pc: 0x7ee096f9048a]
13 6: findIndex [0x7ee70f986d49](this=0x7ee70f9aab71 <JSFunction c (sfi = 0x7ee70f9aa739)>#4#,0x7ee70f9aab71 <JSFunction c (sfi = 0x7ee70f9aa739)>#4#)
14 7: b [0x7ee70f9aab09] [/Users/xiaozhouzhou/Desktop/poc.js:5] [bytecode=0x7ee70f9aaff9 offset=37](this=0x7ee7c4a83721 <JSGlobal Object>#3#)
15 8: arguments adaptor frame: 3->0
16 9: find [0x7ee70f986d11](this=0x7ee7c4ab51e1 <JSArray[10]>#5#,0x7ee70f9aab09 <JSFunction b (sfi = 0x7ee70f9aa6c9)>#6#)
17 10: d [0x7ee70f9aabd9] [/Users/xiaozhouzhou/Desktop/poc.js:17] [bytecode=0x7ee70f9aad49 offset=53](this=0x7ee7c4a83721 <JSGlobal Object>#3#)
18 11: arguments adaptor frame: 3->0
19 12: StubFrame [pc: 0x7ee096f74952]
20 13: filter [0x7ee70f986ed1](this=0x7ee70f9ab111 <String[10]: aaaaaaaaaa>,0x7ee70f9aabd9 <JSFunction d (sfi = 0x7ee70f9aa7a9)>#7#)
21 14: c [0x7ee70f9aab71] [/Users/xiaozhouzhou/Desktop/poc.js:9] [bytecode=0x7ee70f9ab259 offset=33](this=0x7ee7c4a83721 <JSGlobal Object>#3#,arg1=0x7ee7ad1826f1 <undefined>)
22 15: arguments adaptor frame: 3->1
23 16: StubFrame [pc: 0x7ee096f9048a]
24 17: findIndex [0x7ee70f986d49](this=0x7ee70f9aab71 <JSFunction c (sfi = 0x7ee70f9aa739)>#4#,0x7ee70f9aab71 <JSFunction c (sfi = 0x7ee70f9aa739)>#4#)
25 18: b [0x7ee70f9aab09] [/Users/xiaozhouzhou/Desktop/poc.js:5] [bytecode=0x7ee70f9aaff9 offset=37](this=0x7ee7c4a83721 <JSGlobal Object>#3#)
26 19: arguments adaptor frame: 3->0
27 20: find [0x7ee70f986d11](this=0x7ee7c4ab5111 <JSArray[10]>#8#,0x7ee70f9aab09 <JSFunction b (sfi = 0x7ee70f9aa6c9)>#6#)
28 21: d [0x7ee70f9aabd9] [/Users/xiaozhouzhou/Desktop/poc.js:17] [bytecode=0x7ee70f9aad49 offset=53](this=0x7ee7c4a83721 <JSGlobal Object>#3#)
29 22: arguments adaptor frame: 3->0
30 23: StubFrame [pc: 0x7ee096f74952]
31 24: filter [0x7ee70f986ed1](this=0x7ee70f9ab111 <String[10]: aaaaaaaaaa>,0x7ee70f9aabd9 <JSFunction d (sfi = 0x7ee70f9aa7a9)>#7#)
32 25: c [0x7ee70f9aab71] [/Users/xiaozhouzhou/Desktop/poc.js:9] [bytecode=0x7ee70f9ab259 offset=33](this=0x7ee7c4a83721 <JSGlobal Object>#3#,arg1=0x7ee7ad1826f1 <undefined>)
33 26: arguments adaptor frame: 3->1
34 27: StubFrame [pc: 0x7ee096f9048a]
35 28: findIndex [0x7ee70f986d49](this=0x7ee70f9aab71 <JSFunction c (sfi = 0x7ee70f9aa739)>#4#,0x7ee70f9aab71 <JSFunction c (sfi = 0x7ee70f9aa739)>#4#)
...
,
May 28 2018
Automatically applying components based on crash stacktrace and information from OWNERS files. If this is incorrect, please apply the Test-Predator-Wrong-Components label.
,
May 30 2018
,
May 30 2018
ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=6572697476399104.
,
Jun 1 2018
ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://clusterfuzz.com/testcase?key=5926303690391552.
,
Jun 4 2018
ishell: Just wondering if you had a chance to look at this?
,
Jun 5 2018
,
Jun 5 2018
,
Jun 5 2018
Minimal repro:
let o = {
length: Infinity
};
Array.prototype.reduceRight.call(o, ()=>{});
From what I see this is not a security issue and just a correctness bug since we don't convert the input back to a string anymore.
,
Jun 5 2018
,
Jun 5 2018
I'm not yet sure it's not a security issue. The minimal repro worked for me, and in addition to the JS stacktrace, I got this native stack trace: Received signal 4 ILL_ILLOPN 7f7c6faa7932 #0 0x7f7c96e81bcd base::debug::StackTrace::StackTrace() #1 0x7f7c96bca4bc base::debug::StackTrace::StackTrace() #2 0x7f7c96e81624 base::debug::(anonymous namespace)::StackDumpSignalHandler() #3 0x7f7c7837f0c0 <unknown> #4 0x7f7c6faa7932 v8::base::OS::Abort() #5 0x7f7c8193d2be v8::internal::__RT_impl_Runtime_AbortJS() #6 0x7f7c8193cd28 v8::internal::Runtime_AbortJS() #7 0x326c9bfb4284 <unknown> r8: 00007ffd675311e8 r9: 0000000000000400 r10: 0000000000000073 r11: 00007f7c74254e00 r12: 00003333d1697271 r13: 00002052af219001 r14: 00007ffd67531900 r15: 0000000000000001 di: 00002052aef1a098 si: 0000000000001e1f bp: 00007ffd675317e0 bx: 00002052af219020 dx: 00002052b00ad600 ax: 0000000000000001 cx: 00000000000001e1 sp: 00007ffd67531788 ip: 00007f7c6faa7932 efl: 0000000000010202 cgf: 002b000000000033 erf: 0000000000000000 trp: 0000000000000006 msk: 0000000000000000 cr2: 0000000000000000 [end of stack trace] `SIGILL` suggests to me that the program tried to execute junk (`rip` is clobbered or corrupted), which sounds like a security concern. I could be wrong, but.
,
Jun 6 2018
I'm currently rewriting the checks and adding some more detailed check to see whether lookup happens properly. In the submitted poc.js we most definitely run out of stackspace in optdebug, wasn't patient enough yet to see what it does in release. I will check in detail again, here's the current finding: The index (k in the CSA stub) is used for a lookup, the failing check was ensuring that we don't have to do ToString conversion. k is guaranteed to be a Number from the get-go, so the ToString conversion is non-observable. k is then used to check (HasProperty) and to get the value (GetProperty) from the receiver which both to TryToName [1] internally. The fast-path avoids the conversion for ArrayIndices and uses k directly on an elements-only lookup. In all other cases we convert k to a String and do a normal property lookup. [1] https://cs.chromium.org/chromium/src/v8/src/code-stub-assembler.cc?type=cs&q=TryToName&sq=package:chromium&g=0&l=6944
,
Jun 7 2018
#12: Thanks. What's the worst outcome? out-of-bounds read/read of an invalid property or array index?
,
Jun 13 2018
Sorry for the late reply, was OOO. From my current assessment there is no negative outcome security-wise. This was just a overzealous check. Even if the check failed, the input number would be properly converted for a lookup since we rely on the generic HasProperty helper [1] which falls back to the HasProperty runtime function [2] which does proper string conversion for all inputs. [1] https://cs.chromium.org/chromium/src/v8/src/code-stub-assembler.cc?type=cs&q=HasProperty&g=0&l=10868 [2] https://cs.chromium.org/chromium/src/v8/src/runtime/runtime-object.cc?type=cs&q=Runtime_HasProperty&g=0&l=647
,
Jun 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/9718d079e930bc3880c9ca8f2fc3807de3d98f68 commit 9718d079e930bc3880c9ca8f2fc3807de3d98f68 Author: Camillo Bruni <cbruni@chromium.org> Date: Wed Jun 13 15:38:01 2018 [CSA] Fix Array.prototype.reduceRight CAS_ASSERT - Add typed IsHeapNumberPositive, IsNumberNonNegativeSafeInteger, IsInteger, IsSafeInteger and IsHeapNumberUint32 helpers on CodeStubAssembler - Type NumberIsInteger and NumberIsSafeInteger builtin Bug: chromium:847204 , v8:6949 Change-Id: I27d3ab79bd17312c223209ed0b221c174024126e Reviewed-on: https://chromium-review.googlesource.com/1087961 Commit-Queue: Camillo Bruni <cbruni@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Cr-Commit-Position: refs/heads/master@{#53707} [modify] https://crrev.com/9718d079e930bc3880c9ca8f2fc3807de3d98f68/src/builtins/builtins-array-gen.cc [modify] https://crrev.com/9718d079e930bc3880c9ca8f2fc3807de3d98f68/src/builtins/builtins-array-gen.h [modify] https://crrev.com/9718d079e930bc3880c9ca8f2fc3807de3d98f68/src/builtins/builtins-number-gen.cc [modify] https://crrev.com/9718d079e930bc3880c9ca8f2fc3807de3d98f68/src/code-stub-assembler.cc [modify] https://crrev.com/9718d079e930bc3880c9ca8f2fc3807de3d98f68/src/code-stub-assembler.h [modify] https://crrev.com/9718d079e930bc3880c9ca8f2fc3807de3d98f68/test/cctest/test-code-stub-assembler.cc
,
Jun 14 2018
Given #14 I'm tempted to just turn this into a normal type=bug or security-impact-none. cbruni, can you confirm, and do that, or I'll re-triage tomorrow. Thanks.
,
Jun 14 2018
Changed to type=bug according to #14 and #16 since there is no security impact.
,
Jun 15 2018
The NextAction date has arrived: 2018-06-15
,
Jun 15 2018
,
Sep 21
This bug has been closed for more than 14 weeks. Removing security view restrictions. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by ClusterFuzz
, May 28 2018