| Summary: | [YARR JIT] Crash on overflow when compiling /(a{1000000000}b{1000000000}|c{1000000000}|)d{1000000000}e{1000000000}/.test(); | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Mark Lam <mark.lam> | ||||||
| Component: | JavaScriptCore | Assignee: | Michael Saboff <msaboff> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | ews-watchlist, keith_miller, msaboff, saam, tzagallo, webkit-bug-importer, ysuzuki | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Mark Lam
2020-12-23 16:16:07 PST
Created attachment 416731 [details]
proposed patch.
Comment on attachment 416731 [details]
proposed patch.
Seems sane.
Thanks. I would still like Michael to take a look before I land, especially on whether I should be using a different error code. Comment on attachment 416731 [details]
proposed patch.
I spent some time trying the patch. The test case should not cause an overflow. The patch found a bug and through code inspection I found another related bug. If you look at lines ~3107..3111, you'll notice that is "if (!isBegin)" is true, we add the offset from the prior op before subtracting the checked amount for the current op. This is where we overflow with the test and this patch. We should be subtracting the current checked amount before adding the prior. When I made this fix, the offset never overflowed. When I adjust the test to have combined sounds >= 2^32, we overflow in the parser.
There is a similar "add before subtract" offset case in lines 2801..2805.
In a local build, I change both cases to have the subtract before the add and ran all JSC tests with no issues.
We can land this patch, but it is more of a canary for newly introduced bugs in YARR JIT code (after I land my changes). If we do, we should restructure the code to assert no overflow.
Michael is going to implement the real fix. We do some checked math in the Yarr JIT compiler in the wrong order, addition before subtraction causing checked arithmetic overflows. Created attachment 421381 [details]
Patch
Comment on attachment 421381 [details]
Patch
r=me
Committed r273371: <https://commits.webkit.org/r273371> All reviewed patches have been landed. Closing bug and clearing flags on attachment 421381 [details]. |