Security: V8: JIT: Type confusion in NodeProperties::InferReceiverMaps |
|||||||||||
Issue descriptionhttps://cs.chromium.org/chromium/src/v8/src/compiler/node-properties.cc?rcl=df84e87191022bf6914f9570069908f10b303245&l=416 Here's a snippet of NodeProperties::InferReceiverMaps. case IrOpcode::kJSCreate: { if (IsSame(receiver, effect)) { HeapObjectMatcher mtarget(GetValueInput(effect, 0)); HeapObjectMatcher mnewtarget(GetValueInput(effect, 1)); if (mtarget.HasValue() && mnewtarget.HasValue()) { Handle<JSFunction> original_constructor = Handle<JSFunction>::cast(mnewtarget.Value()); if (original_constructor->has_initial_map()) { Handle<Map> initial_map(original_constructor->initial_map()); if (initial_map->constructor_or_backpointer() == *mtarget.Value()) { *maps_return = ZoneHandleSet<Map>(initial_map); return result; } } } // We reached the allocation of the {receiver}. return kNoReceiverMaps; } break; } "mnewtarget" is expected to be a constructor which also can be of type JSBoundFunction. But "mnewtarget" is always cast to JSFunction leading to type confusion. The PoC seems not to crash in release mode. Debug mode log: # # Fatal error in ../../src/objects-inl.h, line 566 # Check failed: !v8::internal::FLAG_enable_slow_asserts || (object->IsJSFunction()). # ==== C stack trace =============================== /v8/out.gn/x64.debug/./libv8_libbase.so(v8::base::debug::StackTrace::StackTrace()+0x1e) [0x7f4623e1043e] /v8/out.gn/x64.debug/./libv8_libplatform.so(+0x30907) [0x7f4623db3907] /v8/out.gn/x64.debug/./libv8_libbase.so(V8_Fatal(char const*, int, char const*, ...)+0x1bd) [0x7f4623df876d] /v8/out.gn/x64.debug/./libv8.so(v8::internal::JSFunction::cast(v8::internal::Object*)+0x64) [0x7f46226584a4] /v8/out.gn/x64.debug/./libv8.so(v8::internal::Handle<v8::internal::JSFunction> const v8::internal::Handle<v8::internal::JSFunction>::cast<v8::internal::JSFunction>(v8::internal::Handle<v8::internal::JSFunction>)+0x23) [0x7f4622651173] /v8/out.gn/x64.debug/./libv8.so(v8::internal::compiler::NodeProperties::InferReceiverMaps(v8::internal::compiler::Node*, v8::internal::compiler::Node*, v8::internal::ZoneHandleSet<v8::internal::Map>*)+0x435) [0x7f4622c24a75] /v8/out.gn/x64.debug/./libv8.so(v8::internal::compiler::JSNativeContextSpecialization::InferReceiverMaps(v8::internal::compiler::Node*, v8::internal::compiler::Node*, std::__1::vector<v8::internal::Handle<v8::internal::Map>, std::__1::allocator<v8::internal::Handle<v8::internal::Map> > >*)+0x50) [0x7f4622b8b820] /v8/out.gn/x64.debug/./libv8.so(v8::internal::compiler::JSNativeContextSpecialization::ExtractReceiverMaps(v8::internal::compiler::Node*, v8::internal::compiler::Node*, v8::internal::FeedbackNexus const&, std::__1::vector<v8::internal::Handle<v8::internal::Map>, std::__1::allocator<v8::internal::Handle<v8::internal::Map> > >*)+0x202) [0x7f4622b82632] /v8/out.gn/x64.debug/./libv8.so(v8::internal::compiler::JSNativeContextSpecialization::ReduceNamedAccessFromNexus(v8::internal::compiler::Node*, v8::internal::compiler::Node*, v8::internal::FeedbackNexus const&, v8::internal::Handle<v8::internal::Name>, v8::internal::compiler::AccessMode)+0x2e6) [0x7f4622b822b6] /v8/out.gn/x64.debug/./libv8.so(v8::internal::compiler::JSNativeContextSpecialization::ReduceJSStoreNamed(v8::internal::compiler::Node*)+0x298) [0x7f4622b7c2c8] /v8/out.gn/x64.debug/./libv8.so(v8::internal::compiler::JSNativeContextSpecialization::Reduce(v8::internal::compiler::Node*)+0x11f) [0x7f4622b78f7f] /v8/out.gn/x64.debug/./libv8.so(v8::internal::compiler::GraphReducer::Reduce(v8::internal::compiler::Node*)+0x285) [0x7f4622ad8c55] /v8/out.gn/x64.debug/./libv8.so(v8::internal::compiler::GraphReducer::ReduceTop()+0x44f) [0x7f4622ad874f] /v8/out.gn/x64.debug/./libv8.so(v8::internal::compiler::GraphReducer::ReduceNode(v8::internal::compiler::Node*)+0x1bc) [0x7f4622ad7cfc] /v8/out.gn/x64.debug/./libv8.so(v8::internal::compiler::GraphReducer::ReduceGraph()+0x2d) [0x7f4622ad89bd] /v8/out.gn/x64.debug/./libv8.so(v8::internal::compiler::InliningPhase::Run(v8::internal::compiler::PipelineData*, v8::internal::Zone*)+0x58a) [0x7f4622c46e2a] PoC: // Flags: --allow-natives-syntax --enable_slow_asserts class Base { constructor() { this.x = 1; } } class Derived extends Base { constructor() { // JSCreate emitted I guess. super(); } } let bound = Object.bind(); Reflect.construct(Derived, [], bound); // Feed a bound function as new.target to the profiler, so HeapObjectMatcher can find it. %OptimizeFunctionOnNextCall(Derived); new Derived(); This bug is subject to a 90 day disclosure deadline. After 90 days elapse or a patch has been made broadly available, the bug report will become visible to the public.
,
Jan 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/e272a2f722422651cf5bbbe0168702ee5d38cfe8 commit e272a2f722422651cf5bbbe0168702ee5d38cfe8 Author: Benedikt Meurer <bmeurer@chromium.org> Date: Mon Jan 15 06:56:47 2018 [turbofan] Fix type confusion in NodeProperties::InferReceiverMaps. For JSCreate nodes with constant inputs we cannot simply assume that the new.target input is a JSFunction, since it can essentially be any JSReceiver that is a constructor, i.e. it can also be a JSBoundFunction. Bug: chromium:801627 Change-Id: Ia37bf9c0a751e4665e1167a3771fbe166473c979 Reviewed-on: https://chromium-review.googlesource.com/866493 Reviewed-by: Jaroslav Sevcik <jarin@chromium.org> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org> Cr-Commit-Position: refs/heads/master@{#50563} [modify] https://crrev.com/e272a2f722422651cf5bbbe0168702ee5d38cfe8/src/compiler/node-properties.cc [add] https://crrev.com/e272a2f722422651cf5bbbe0168702ee5d38cfe8/test/mjsunit/regress/regress-crbug-801627.js
,
Jan 15 2018
,
Jan 15 2018
This bug requires manual review: We are only 7 days from stable. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 15 2018
,
Jan 16 2018
Pls apply appropriate OSs label.
,
Jan 16 2018
,
Jan 16 2018
+awhalley@
,
Jan 17 2018
,
Jan 17 2018
Please merge this today before 3:00PM PST
,
Jan 18 2018
Ping!
,
Jan 19 2018
Sorry, missed the notification. Will merge now.
,
Jan 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/746005554cf2fff6370399dd62cbd968736de230 commit 746005554cf2fff6370399dd62cbd968736de230 Author: Benedikt Meurer <bmeurer@google.com> Date: Fri Jan 19 07:33:19 2018 Merged: [turbofan] Fix type confusion in NodeProperties::InferReceiverMaps. Revision: e272a2f722422651cf5bbbe0168702ee5d38cfe8 BUG= chromium:801627 LOG=N NOTRY=true NOPRESUBMIT=true NOTREECHECKS=true R=yangguo@chromium.org Change-Id: Ic987c9f76f2c2974d107f2bb9b1ee5988bd50354 Reviewed-on: https://chromium-review.googlesource.com/874371 Reviewed-by: Yang Guo <yangguo@chromium.org> Cr-Commit-Position: refs/branch-heads/6.4@{#76} Cr-Branched-From: 0407506af3d9d7e2718be1d8759296165b218fcf-refs/heads/6.4.388@{#1} Cr-Branched-From: a5fc4e085ee543cb608eb11034bc8f147ba388e1-refs/heads/master@{#49724} [modify] https://crrev.com/746005554cf2fff6370399dd62cbd968736de230/src/compiler/node-properties.cc [add] https://crrev.com/746005554cf2fff6370399dd62cbd968736de230/test/mjsunit/regress/regress-crbug-801627.js
,
Jan 19 2018
,
Jan 24 2018
Is this need a merge to M65? If yes, pls request a merge. Thank you.
,
Jan 24 2018
This is already on 65
,
Apr 23 2018
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 bmeu...@chromium.org
, Jan 15 2018Owner: bmeu...@chromium.org
Status: Assigned (was: Unconfirmed)