FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

wasi: relax WebAssembly.Instance type check · nodejs/node@383c5b3 · GitHub

/ node Public

Commit 383c5b3

Browse files
bnoordhuis
authored and
codebytere
committed
wasi: relax WebAssembly.Instance type check
Instances coming from different VM contexts don't pass `instanceof` type checks because each context has its own copy of the built-in globals. After review of the relevant code it seems like it should be safe to relax the type check and that is what this commit does: `wasi.start()` now accepts any input that walks and quacks like a WebAssembly.Instance or WebAssembly.Memory instance. Fixes: #33415 PR-URL: #33431 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
1 parent 78734c2 commit 383c5b3

2 files changed

Lines changed: 88 additions & 9 deletions

File tree

‎lib/wasi.js‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
'use strict';
2-
/* global WebAssembly */
32
const {
43
ArrayPrototypeMap,
54
ArrayPrototypePush,
@@ -12,6 +11,7 @@ const {
1211
ERR_WASI_ALREADY_STARTED
1312
} = require('internal/errors').codes;
1413
const { emitExperimentalWarning } = require('internal/util');
14+
const { isArrayBuffer } = require('internal/util/types');
1515
const {
1616
validateArray,
1717
validateBoolean,
@@ -70,10 +70,7 @@ class WASI {
7070
}
7171

7272
start(instance) {
73-
if (!(instance instanceof WebAssembly.Instance)) {
74-
throw new ERR_INVALID_ARG_TYPE(
75-
'instance', 'WebAssembly.Instance', instance);
76-
}
73+
validateObject(instance, 'instance');
7774

7875
const exports = instance.exports;
7976

@@ -91,9 +88,19 @@ class WASI {
9188
'instance.exports._initialize', 'undefined', _initialize);
9289
}
9390

94-
if (!(memory instanceof WebAssembly.Memory)) {
91+
// WASI::_SetMemory() in src/node_wasi.cc only expects that |memory| is
92+
// an object. It will try to look up the .buffer property when needed
93+
// and fail with UVWASI_EINVAL when the property is missing or is not
94+
// an ArrayBuffer. Long story short, we don't need much validation here
95+
// but we type-check anyway because it helps catch bugs in the user's
96+
// code early.
97+
validateObject(memory, 'instance.exports.memory');
98+
99+
if (!isArrayBuffer(memory.buffer)) {
95100
throw new ERR_INVALID_ARG_TYPE(
96-
'instance.exports.memory', 'WebAssembly.Memory', memory);
101+
'instance.exports.memory.buffer',
102+
['WebAssembly.Memory'],
103+
memory.buffer);
97104
}
98105

99106
if (this[kStarted]) {

‎test/wasi/test-wasi-start-validation.js‎

Lines changed: 74 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
const common = require('../common');
55
const assert = require('assert');
6+
const vm = require('vm');
67
const { WASI } = require('wasi');
78

89
const fixtures = require('../common/fixtures');
@@ -15,7 +16,10 @@ const bufferSource = fixtures.readSync('simple.wasm');
1516

1617
assert.throws(
1718
() => { wasi.start(); },
18-
{ code: 'ERR_INVALID_ARG_TYPE', message: /\bWebAssembly\.Instance\b/ }
19+
{
20+
code: 'ERR_INVALID_ARG_TYPE',
21+
message: /"instance" argument must be of type object/
22+
}
1923
);
2024
}
2125

@@ -87,11 +91,79 @@ const bufferSource = fixtures.readSync('simple.wasm');
8791
() => { wasi.start(instance); },
8892
{
8993
code: 'ERR_INVALID_ARG_TYPE',
90-
message: /"instance\.exports\.memory" property .+ WebAssembly\.Memory/
94+
message: /"instance\.exports\.memory" property must be of type object/
95+
}
96+
);
97+
}
98+
99+
{
100+
// Verify that a non-ArrayBuffer memory.buffer is rejected.
101+
const wasi = new WASI({});
102+
const wasm = await WebAssembly.compile(bufferSource);
103+
const instance = await WebAssembly.instantiate(wasm);
104+
105+
Object.defineProperty(instance, 'exports', {
106+
get() {
107+
return {
108+
_start() {},
109+
memory: {},
110+
};
111+
}
112+
});
113+
// The error message is a little white lie because any object
114+
// with a .buffer property of type ArrayBuffer is accepted,
115+
// but 99% of the time a WebAssembly.Memory object is used.
116+
assert.throws(
117+
() => { wasi.start(instance); },
118+
{
119+
code: 'ERR_INVALID_ARG_TYPE',
120+
message: /"instance\.exports\.memory\.buffer" property must be an WebAssembly\.Memory/
91121
}
92122
);
93123
}
94124

125+
{
126+
// Verify that an argument that duck-types as a WebAssembly.Instance
127+
// is accepted.
128+
const wasi = new WASI({});
129+
const wasm = await WebAssembly.compile(bufferSource);
130+
const instance = await WebAssembly.instantiate(wasm);
131+
132+
Object.defineProperty(instance, 'exports', {
133+
get() {
134+
return {
135+
_start() {},
136+
memory: { buffer: new ArrayBuffer(0) },
137+
};
138+
}
139+
});
140+
wasi.start(instance);
141+
}
142+
143+
{
144+
// Verify that a WebAssembly.Instance from another VM context is accepted.
145+
const wasi = new WASI({});
146+
const instance = await vm.runInNewContext(`
147+
(async () => {
148+
const wasm = await WebAssembly.compile(bufferSource);
149+
const instance = await WebAssembly.instantiate(wasm);
150+
151+
Object.defineProperty(instance, 'exports', {
152+
get() {
153+
return {
154+
_start() {},
155+
memory: new WebAssembly.Memory({ initial: 1 })
156+
};
157+
}
158+
});
159+
160+
return instance;
161+
})()
162+
`, { bufferSource });
163+
164+
wasi.start(instance);
165+
}
166+
95167
{
96168
// Verify that start() can only be called once.
97169
const wasi = new WASI({});

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL