Bug 196746 - We should clear m_needsOverflowCheck when hitting an exception in defineProperties in ObjectConstructor.cpp
Summary: We should clear m_needsOverflowCheck when hitting an exception in definePrope...
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Robin Morisset
Keywords: InRadar
Depends on:
Reported: 2019-04-09 15:02 PDT by Robin Morisset
Modified: 2019-04-10 11:05 PDT (History)
7 users (show)

See Also:

Patch (3.77 KB, patch)
2019-04-09 15:29 PDT, Robin Morisset
ysuzuki: review+
Details | Formatted Diff | Diff
Patch for landing (3.76 KB, patch)
2019-04-10 10:47 PDT, Robin Morisset
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Robin Morisset 2019-04-09 15:02:04 PDT
Currently we correctly do so when toPropertyDescriptor fails, but not when properties->get(exec, propertyNames[i]); does.
This can turn an OOM into a crash, because of the check in ~MarkedArgumentBuffer.
The fix is a trivial call to markBuffer.overflowCheckNotNeeded() on that path.
Comment 1 Robin Morisset 2019-04-09 15:02:21 PDT
Comment 2 Robin Morisset 2019-04-09 15:29:54 PDT
Created attachment 367074 [details]
Comment 3 EWS Watchlist 2019-04-09 15:31:32 PDT
Attachment 367074 [details] did not pass style-queue:

ERROR: Source/JavaScriptCore/ChangeLog:8:  Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: buffer overflow  [changelog/unwantedsecurityterms] [3]
ERROR: Source/JavaScriptCore/runtime/ObjectConstructor.cpp:614:  Missing space before ( in while(  [whitespace/parens] [5]
Total errors found: 2 in 4 files

If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Yusuke Suzuki 2019-04-09 18:18:40 PDT
Comment on attachment 367074 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=367074&action=review

r=me with nit.

> Source/JavaScriptCore/runtime/ObjectConstructor.cpp:620

We can use `{ }` instead of `jsNull()`.
Comment 5 Robin Morisset 2019-04-10 10:47:16 PDT
Created attachment 367138 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2019-04-10 11:05:06 PDT
Comment on attachment 367138 [details]
Patch for landing

Clearing flags on attachment: 367138

Committed r244136: <https://trac.webkit.org/changeset/244136>
Comment 7 WebKit Commit Bot 2019-04-10 11:05:07 PDT
All reviewed patches have been landed.  Closing bug.