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
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

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. ***