Bug 175693

Summary:

WebAssembly: const in unreachable code decoded incorrectly, erroneously rejects binary as invalid

Product:

WebKit

Reporter:

Alon Zakai <alonzakai>

Component:

JavaScriptCore

Assignee:

JF Bastien <jfbastien>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, buildbot, commit-queue, jfbastien, keith_miller, mark.lam, msaboff, product-security, saam, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Linux

Attachments:

Description	Flags
js script	none
wasm file	none
patch	saam: review+, saam: commit-queue-
patch	commit-queue: commit-queue-
patch	none

Alon Zakai

Reported 2017-08-17 16:08:53 PDT

Created attachment 318433 [details] js script Running jsc b.js -- b.wasm it reports Exception: Error: WebAssembly.Module doesn't parse at byte 287 / 321: can't get immediate for I64Const in unreachable context, in function at index 0 (evaluating 'new WebAssembly.Module(binary)') Module@[native code] global code@b.js:22:63 However the wasm binary is valid (accepted by v8, sm, binaryen, wabt).

Attachments
js script (1.12 KB, application/x-javascript) 2017-08-17 16:08 PDT, Alon Zakai	no flags	Details
wasm file (456 bytes, application/octet-stream) 2017-08-17 16:09 PDT, Alon Zakai	no flags	Details
patch (2.35 KB, patch) 2017-08-17 16:43 PDT, JF Bastien	saam: review+ saam: commit-queue-	Details Formatted Diff Diff
patch (6.27 KB, patch) 2017-08-17 16:58 PDT, JF Bastien	commit-queue: commit-queue-	Details Formatted Diff Diff
patch (6.27 KB, patch) 2017-08-17 17:08 PDT, JF Bastien	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2017-08-17 16:09:14 PDT

<rdar://problem/33952443>

Alon Zakai

Comment 2 2017-08-17 16:09:15 PDT

Created attachment 318434 [details] wasm file

JF Bastien

Comment 3 2017-08-17 16:31:17 PDT

I believe this is a benign bug because unreachable code shouldn't occur anyways. It's still a bug we should fix, though.

JF Bastien

Comment 4 2017-08-17 16:32:17 PDT

It looks like the i64.const causing the issue is: 42 80 80 80 80 80 80 80 80 80 | i64.const -9223372036854775808

JF Bastien

Comment 5 2017-08-17 16:33:38 PDT

Well this is our bug: // one immediate cases case I32Const: case I64Const: // <------ derp case SetLocal: case GetLocal: case TeeLocal: case GetGlobal: case SetGlobal: case Br: case BrIf: case Call: { uint32_t unused; WASM_PARSER_FAIL_IF(!parseVarUInt32(unused), "can't get immediate for ", m_currentOpcode, " in unreachable context"); return { }; } This is benign, not a security issue.

JF Bastien

Comment 6 2017-08-17 16:43:33 PDT

Created attachment 318440 [details] patch

Saam Barati

Comment 7 2017-08-17 16:44:39 PDT

Comment on attachment 318440 [details] patch please add test

JF Bastien

Comment 8 2017-08-17 16:58:50 PDT

Created attachment 318443 [details] patch Add test, and also handle i32.const / i64.const as signed.

WebKit Commit Bot

Comment 9 2017-08-17 16:59:52 PDT

Comment on attachment 318443 [details] patch Rejecting attachment 318443 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 318443, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in JSTests/ChangeLog contains OOPS!. Full output: http://webkit-queues.webkit.org/results/4333352

JF Bastien

Comment 10 2017-08-17 17:08:59 PDT

Created attachment 318444 [details] patch Fix OOPS.

WebKit Commit Bot

Comment 11 2017-08-17 18:03:28 PDT

The commit-queue encountered the following flaky tests while processing attachment 318444 [details]: svg/animations/smil-leak-list-property-instances.svg bug 175701 (author: sabouhallawa@apple.com) The commit-queue is continuing to process your patch.

WebKit Commit Bot

Comment 12 2017-08-17 18:04:03 PDT

Comment on attachment 318444 [details] patch Clearing flags on attachment: 318444 Committed r220894: <http://trac.webkit.org/changeset/220894>

WebKit Commit Bot

Comment 13 2017-08-17 18:04:06 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.