Security: V8: AwaitedPromise update bug |
|||||||||||||||||||||||
Issue descriptionHere's a snippet of AsyncGeneratorReturn. (https://cs.chromium.org/chromium/src/v8/src/builtins/builtins-async-generator-gen.cc?rcl=bcd1365cf7fac0d7897c43b377c143aae2d22f92&l=650) Node* const context = Parameter(Descriptor::kContext); Node* const outer_promise = LoadPromiseFromAsyncGeneratorRequest(req); Node* const promise = Await(context, generator, value, outer_promise, AwaitContext::kLength, init_closure_context, var_on_resolve.value(), var_on_reject.value(), is_caught); CSA_SLOW_ASSERT(this, IsGeneratorNotSuspendedForAwait(generator)); StoreObjectField(generator, JSAsyncGeneratorObject::kAwaitedPromiseOffset, promise); The Await methods calls ResolveNativePromise which calls InternalResolvePromise which can invoke a user JavaScript code through a "then" getter. If the AwaitedPromise is replaced by the user script, the AwaitedPromise will be immediately overwritten after the call to Await, this may lead the generator to an incorrect state. PoC: async function* asyncGenerator() { } let gen = asyncGenerator(); gen.return({ get then() { delete this.then; gen.next(); } }); Log in debug mode: abort: CSA_ASSERT failed: IsNotUndefined(request) [../../src/builtins/builtins-async-generator-gen.cc:328] ==== JS stack trace ========================================= Security context: 0x2b29083a3a71 <JSObject>#0# 2: /* anonymous */(this=0x19b7b0603721 <JSGlobal Object>#1#,0x19b7b060d139 <Object map = 0x189055388c91>#2#) ==== Details ================================================ [2]: /* anonymous */(this=0x19b7b0603721 <JSGlobal Object>#1#,0x19b7b060d139 <Object map = 0x189055388c91>#2#) { // optimized frame --------- s o u r c e c o d e --------- <No Source> ----------------------------------------- } ==== Key ============================================ #0# 0x2b29083a3a71: 0x2b29083a3a71 <JSObject> #1# 0x19b7b0603721: 0x19b7b0603721 <JSGlobal Object> #2# 0x19b7b060d139: 0x19b7b060d139 <Object map = 0x189055388c91> ===================== Received signal 4 ILL_ILLOPN 7fb143ae2781 ==== C stack trace =============================== [0x7fb143ae643e] [0x7fb143ae6395] [0x7fb1436ce390] [0x7fb143ae2781] [0x7fb1430f23ae] [0x7fb1430f1ef7] [0x1c8e08204384] [end of stack trace] Illegal instruction 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 25 2018
I'm guessing the iOS-only label was accidental? neis: Can you please take a look?
,
Jan 25 2018
,
Jan 25 2018
,
Jan 25 2018
,
Jan 25 2018
,
Jan 25 2018
,
Jan 25 2018
,
Jan 26 2018
Caitlin, please have a look. It seems that not having an explicit "awaiting-return" state is problematic. From what I can tell, the write to kAwaitedPromiseOffset in AsyncGeneratorReturn happens too late, because the Await can trigger a reentrance of this code.
,
Jan 26 2018
,
Jan 26 2018
If you have cycles to take this, you should. I can assist in small ways, but won’t be back to my normal v8 schedule for some time.
,
Jan 29 2018
,
Jan 29 2018
neis: Can you please help assign a Security_Impact label? Does this affect stable channel?
,
Jan 30 2018
,
Jan 30 2018
,
Jan 31 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/9c4c717b5d1765ff31f773df416115d3dd08de60 commit 9c4c717b5d1765ff31f773df416115d3dd08de60 Author: Georg Neis <neis@chromium.org> Date: Wed Jan 31 07:43:28 2018 Fix bug in async generators. Async generators didn't correctly handle the situation where one calls .return on a suspended-at-start async generator and passes a promise-like object whose awaiting causes a new request to the generator. Bug: chromium:805729 Change-Id: I4da13ab5bd97f8c2a2c5373242a2d5e2ab0f7f10 Reviewed-on: https://chromium-review.googlesource.com/891231 Reviewed-by: Caitlin Potter <caitp@igalia.com> Reviewed-by: Jaroslav Sevcik <jarin@chromium.org> Commit-Queue: Georg Neis <neis@chromium.org> Cr-Commit-Position: refs/heads/master@{#50974} [modify] https://crrev.com/9c4c717b5d1765ff31f773df416115d3dd08de60/src/builtins/builtins-async-generator-gen.cc [modify] https://crrev.com/9c4c717b5d1765ff31f773df416115d3dd08de60/src/builtins/builtins-object-gen.cc [modify] https://crrev.com/9c4c717b5d1765ff31f773df416115d3dd08de60/src/compiler/access-builder.cc [modify] https://crrev.com/9c4c717b5d1765ff31f773df416115d3dd08de60/src/compiler/access-builder.h [modify] https://crrev.com/9c4c717b5d1765ff31f773df416115d3dd08de60/src/compiler/js-create-lowering.cc [modify] https://crrev.com/9c4c717b5d1765ff31f773df416115d3dd08de60/src/objects-inl.h [modify] https://crrev.com/9c4c717b5d1765ff31f773df416115d3dd08de60/src/objects.h [modify] https://crrev.com/9c4c717b5d1765ff31f773df416115d3dd08de60/src/runtime/runtime-generator.cc [add] https://crrev.com/9c4c717b5d1765ff31f773df416115d3dd08de60/test/mjsunit/regress/regress-805729.js
,
Feb 5 2018
,
Feb 8 2018
,
Mar 6 2018
,
Mar 12 2018
,
Apr 17 2018
,
Apr 25 2018
,
Apr 25 2018
,
May 14 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
,
Jun 20 2018
,
Jun 26 2018
,
Jan 4
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by ClusterFuzz
, Jan 25 2018