Issue metadata
Sign in to add a comment
|
Security: OOB read with CallSite and wasm
Reported by
pim...@live.nl,
Jul 30 2016
|
||||||||||||||||||||||
Issue description
VULNERABILITY DETAILS
A recent commit in V8 [1] ported CallSite to C++. I think there is a typo in `CallSiteConstructor` [2]:
SET_CALLSITE_PROPERTY(obj, call_site_wasm_func_index_symbol, fun);
I guess this should be `fun_index` instead of `fun`. The current code allows for an OOB read because `fun` is controlled by the attacker. If `getFunctionName` is called, then this value is eventually passed to `GetWasmFunctionNameFromTable` (as `func_index`) and a DCHECK is hit [3]:
...
DCHECK(func_index < num_funcs);
int offset = func_names_array->get_int(func_index + 1);
...
Note that fixing the typo (if it is one) can still trigger another DCHECK: replacing the number 99999 with a function literal in the last line below triggers line 61 in builtins-callsite.cc. An error is thrown only for (!function && !wasm), not for (function && wasm). That DCHECK seems harmless, though.
VERSION
I used d8 from [4] with `--expose-wasm`, which should be V8 commit position 38161. Note that it only reproduces with this flag.
On 54.0.2813.0 canary (64-bit) with `--expose-wasm` in `--js-flags`, the renderer hangs. With 99999 replaced by -1, I get the string "\x00\x00\x00\x00".
REPRODUCTION CASE
var module = new WebAssembly.Module(new Uint8Array([0x00, 0x61, 0x73, 0x6d, 0x0b, 0x00, 0x00, 0x00]));
var instance = new WebAssembly.Instance(module);
Error.prepareStackTrace = function(a, b) { return b; };
var stack;
try { throw new Error(); } catch(e) { stack = e.stack; }
stack[0].constructor(instance, 99999).getFunctionName();
[1] https://chromium.googlesource.com/v8/v8/+/c8a0dce96c989ea56632613215aa172352cbf9ec
[2] https://chromium.googlesource.com/v8/v8/+/master/src/builtins/builtins-callsite.cc#67
[3] https://chromium.googlesource.com/v8/v8/+/master/src/wasm/wasm-function-name-table.cc#57
[4] https://storage.googleapis.com/chromium-v8/v8-win32-dbg/full-build-win32_dc78fefb152ec5adab1f4d96734d2633a84a69c4.zip
,
Jul 31 2016
Actually, I now think replacing `fun` by `fun_index` wouldn't fix it, because `fun_index` is also 99999.
,
Aug 1 2016
Thanks for the report! fun vs. fun_index is indeed a typo, fixing ASAP. However the bigger issue seems the be that the user can call our CallSite constructor with arbitrary arguments. Your repro case causes a crash even before the CallSite move to C++. We're currently porting all of our internal Error and CallSite-related JS code to C++; and once that is done, we can make the CallSite constructor inaccessible from JS. Hopefully that should happen within a day or two.
,
Aug 1 2016
,
Aug 1 2016
,
Aug 2 2016
,
Aug 2 2016
Please check whether this can be exploited on the beta and stable channels.
,
Aug 2 2016
Beta channel (Version 53.0.2785.34 beta (64-bit)) is vulnerable:
function EmptyTest() {
"use asm";
function empty() { return 0; }
return {empty: empty};
}
var my_wasm = Wasm.instantiateModuleFromAsm(EmptyTest.toString());
Error.prepareStackTrace = function(a, b) { return b; };
var stack;
try { throw new Error(); } catch(e) { stack = e.stack; }
stack[0].constructor(my_wasm, 99999).getFunctionName();
,
Aug 2 2016
we should add bounds checks to beta and return undefined if oob.
,
Aug 2 2016
Bisected down to 3596cac87ecb107b723aecb9f457c1c9fb5c1df0 (5.2.370). Stable (5.2.361.43) should not be affected.
,
Aug 4 2016
A fix for beta channel is in https://codereview.chromium.org/2199333002/. This already in yesterday's canary.
,
Aug 4 2016
And the fix for ToT: CallSite constructors are made inaccessible from JS in https://codereview.chromium.org/2201823002.
,
Aug 4 2016
Please mark security bugs as fixed as soon as the fix lands, and before requesting merges. This update is based on the merge- labels applied to this issue. Please reopen if this update was incorrect. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 5 2016
Your change meets the bar and is auto-approved for M53 (branch: 2785)
,
Aug 5 2016
,
Aug 5 2016
Please merge your change to M53 branch 2785 before 5:00 PM PT Monday (08/08) so we can take this change in for next week Beta. Thank you.
,
Aug 8 2016
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 8 2016
,
Nov 10 2016
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 rickyz@chromium.org
, Jul 31 2016Labels: Security_Severity-Low Security_Impact-None OS-All
Owner: jgruber@chromium.org
Status: Assigned (was: Unconfirmed)