Bug 214680

Summary: Add exception check for WebCore createRejectedPromiseWithTypeError
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: mark.lam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 214698, 214700    
Bug Blocks:    
Attachments:
Description Flags
Patch mark.lam: review+

Yusuke Suzuki
Reported 2020-07-23 03:41:43 PDT
Add exception check for WebCore createRejectedPromiseWithTypeError
Attachments
Patch (7.72 KB, patch)
2020-07-23 03:42 PDT, Yusuke Suzuki 		mark.lam: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1 2020-07-23 03:42:22 PDT
Created attachment 405030 [details] Patch
Yusuke Suzuki
Comment 2 2020-07-23 03:42:25 PDT
<rdar://problem/65925490>
Mark Lam
Comment 3 2020-07-23 10:58:11 PDT
Comment on attachment 405030 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405030&action=review r=me with fix. > Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp:228 > + auto rejectFunction = promiseConstructor->get(&lexicalGlobalObject, vm.propertyNames->builtinNames().rejectPrivateName()); > + EXCEPTION_ASSERT(!scope.exception() || isTerminatedExecutionException(vm, scope.exception())); Are you sure that getting the property with rejectPrivateName() will never throw? I see that it can be lazily generated using promiseConstructorRejectCodeGenerator(). Can this trigger an OOME / StackOverflow? Regardless, I think you need a RETURN_IF_EXCEPTION() after this. Even if we're seeing a termination exception, we still need to bail.
Yusuke Suzuki
Comment 4 2020-07-23 13:17:33 PDT
Comment on attachment 405030 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405030&action=review >> Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp:228 >> + EXCEPTION_ASSERT(!scope.exception() || isTerminatedExecutionException(vm, scope.exception())); > > Are you sure that getting the property with rejectPrivateName() will never throw? I see that it can be lazily generated using promiseConstructorRejectCodeGenerator(). Can this trigger an OOME / StackOverflow? Regardless, I think you need a RETURN_IF_EXCEPTION() after this. Even if we're seeing a termination exception, we still need to bail. OK, maybe, just using RETURN_IF_EXCEPTION() is better. Changed.
Yusuke Suzuki
Comment 5 2020-07-23 13:37:47 PDT
Committed r264788: <https://trac.webkit.org/changeset/264788>
Yusuke Suzuki
Comment 6 2020-07-23 14:17:08 PDT
Re-opened since this is blocked by bug 214698
Note You need to log in before you can comment on or make changes to this bug.