Bug 182404 - Fix crashes due to mishandling custom sections.
Summary: Fix crashes due to mishandling custom sections.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keith Miller
URL:
Keywords: InRadar
: 182208 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-02-01 13:18 PST by Keith Miller
Modified: 2018-05-01 08:22 PDT (History)
8 users (show)

See Also:


Attachments
Patch (9.62 KB, patch)
2018-02-01 13:43 PST, Keith Miller
no flags Details | Formatted Diff | Diff
Patch for landing (9.71 KB, patch)
2018-02-01 15:22 PST, Keith Miller
keith_miller: commit-queue+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keith Miller 2018-02-01 13:18:22 PST
Fix crashes due to mishandling custom sections.
Comment 1 Keith Miller 2018-02-01 13:43:30 PST
Created attachment 332906 [details]
Patch
Comment 2 Keith Miller 2018-02-01 13:44:18 PST
rdar://problem/36935863
Comment 3 Saam Barati 2018-02-01 14:12:29 PST
Comment on attachment 332906 [details]
Patch

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

r=me

> Source/JavaScriptCore/wasm/WasmModuleParser.cpp:58
> +    // This is  not really a known section

extra space

> Source/JavaScriptCore/wasm/WasmSections.h:55
> +    // It's important that Begin is less than every other section number and that Custom is greater.
> +    // This only works because section numbers are currently monotonically increasing and we don't support
> +    // any custom sections. Also, Begin is not a real section but is used as a marker for validating the ordering
> +    // of sections.
> +    Begin = 0,

static_assert please
Comment 4 JF Bastien 2018-02-01 14:14:00 PST
Comment on attachment 332906 [details]
Patch

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

r=me

> Source/JavaScriptCore/wasm/WasmSections.h:53
> +    // any custom sections. Also, Begin is not a real section but is used as a marker for validating the ordering

We support the "name" custom section.

> Source/JavaScriptCore/wasm/WasmSections.h:77
> +    if (!sectionByte)

Compare to static_cast<uint8_t>(Section::Custom) instead?
Comment 5 Keith Miller 2018-02-01 14:18:42 PST
Comment on attachment 332906 [details]
Patch

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

>> Source/JavaScriptCore/wasm/WasmSections.h:53
>> +    // any custom sections. Also, Begin is not a real section but is used as a marker for validating the ordering
> 
> We support the "name" custom section.

Oh right, I'll change the comment.

>> Source/JavaScriptCore/wasm/WasmSections.h:55
>> +    Begin = 0,
> 
> static_assert please

What static assert are you looking for? Begin < Type?

>> Source/JavaScriptCore/wasm/WasmSections.h:77
>> +    if (!sectionByte)
> 
> Compare to static_cast<uint8_t>(Section::Custom) instead?

That would be incorrect...
Comment 6 Keith Miller 2018-02-01 15:22:45 PST
Created attachment 332914 [details]
Patch for landing
Comment 7 Keith Miller 2018-02-01 20:30:42 PST
Committed r227994: <https://trac.webkit.org/changeset/227994>
Comment 8 Brent Fulgham 2018-05-01 08:22:59 PDT
*** Bug 182208 has been marked as a duplicate of this bug. ***