Skip to content

Commit f5c90b2

Browse files
tniessenRafaelGSS
tniessen
authored and
RafaelGSS
committed
module: fix code injection through export names
createDynamicModule() properly escapes import names, but not export names. In WebAssembly, any string is a valid export name. Importing a WebAssembly module that uses a non-identifier export name leads to either a syntax error in createDynamicModule() or to code injection, that is, to the evaluation of almost arbitrary JavaScript code outside of the WebAssembly module. To address this issue, adopt the same mechanism in createExport() that createImport() already uses. Add tests for both exports and imports. PR-URL: nodejs-private/node-private#461 Backport-PR-URL: nodejs-private/node-private#489 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> CVE-ID: CVE-2023-39333
1 parent a4cb7fc commit f5c90b2

File tree

8 files changed

+81
-7
lines changed

8 files changed

+81
-7
lines changed

‎lib/internal/modules/esm/create_dynamic_module.js‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,13 @@ function createImport(impt, index) {
1717
import.meta.imports[${imptPath}] = $import_${index};`;
1818
}
1919

20-
function createExport(expt) {
21-
const name = `${expt}`;
22-
return `let $${name};
23-
export { $${name} as ${name} };
24-
import.meta.exports.${name} = {
25-
get: () => $${name},
26-
set: (v) => $${name} = v,
20+
function createExport(expt, index) {
21+
const nameStringLit = JSONStringify(expt);
22+
return `let $export_${index};
23+
export { $export_${index} as ${nameStringLit} };
24+
import.meta.exports[${nameStringLit}] = {
25+
get: () => $export_${index},
26+
set: (v) => $export_${index} = v,
2727
};`;
2828
}
2929

‎test/es-module/test-esm-wasm.mjs‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,56 @@ describe('ESM: WASM modules', { concurrency: true }, () => {
2929
strictEqual(code, 0);
3030
});
3131

32+
it('should not allow code injection through export names', async () => {
33+
const { code, stderr, stdout } = await spawnPromisified(execPath, [
34+
'--no-warnings',
35+
'--experimental-wasm-modules',
36+
'--input-type=module',
37+
'--eval',
38+
`import * as wasmExports from ${JSON.stringify(fixtures.fileURL('es-modules/export-name-code-injection.wasm'))};`,
39+
]);
40+
41+
strictEqual(stderr, '');
42+
strictEqual(stdout, '');
43+
strictEqual(code, 0);
44+
});
45+
46+
it('should allow non-identifier export names', async () => {
47+
const { code, stderr, stdout } = await spawnPromisified(execPath, [
48+
'--no-warnings',
49+
'--experimental-wasm-modules',
50+
'--input-type=module',
51+
'--eval',
52+
[
53+
'import { strictEqual } from "node:assert";',
54+
`import * as wasmExports from ${JSON.stringify(fixtures.fileURL('es-modules/export-name-syntax-error.wasm'))};`,
55+
'assert.strictEqual(wasmExports["?f!o:o<b>a[r]"]?.value, 12682);',
56+
].join('\n'),
57+
]);
58+
59+
strictEqual(stderr, '');
60+
strictEqual(stdout, '');
61+
strictEqual(code, 0);
62+
});
63+
64+
it('should properly escape import names as well', async () => {
65+
const { code, stderr, stdout } = await spawnPromisified(execPath, [
66+
'--no-warnings',
67+
'--experimental-wasm-modules',
68+
'--input-type=module',
69+
'--eval',
70+
[
71+
'import { strictEqual } from "node:assert";',
72+
`import * as wasmExports from ${JSON.stringify(fixtures.fileURL('es-modules/import-name.wasm'))};`,
73+
'assert.strictEqual(wasmExports.xor(), 12345);',
74+
].join('\n'),
75+
]);
76+
77+
strictEqual(stderr, '');
78+
strictEqual(stdout, '');
79+
strictEqual(code, 0);
80+
});
81+
3282
it('should emit experimental warning', async () => {
3383
const { code, signal, stderr } = await spawnPromisified(execPath, [
3484
'--experimental-wasm-modules',

‎test/fixtures/es-modules/export-name-code-injection.wasm‎

98 Bytes
Binary file not shown.

‎test/fixtures/es-modules/export-name-code-injection.wat‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
;; Compiled using the WebAssembly Binary Toolkit (https://github.com/WebAssembly/wabt)
2+
;; $ wat2wasm export-name-code-injection.wat
3+
4+
(module
5+
(global $0 i32 (i32.const 123))
6+
(global $1 i32 (i32.const 456))
7+
(export ";import.meta.done=()=>{};console.log('code injection');{/*" (global $0))
8+
(export "/*/$;`//" (global $1)))

‎test/fixtures/es-modules/export-name-syntax-error.wasm‎

37 Bytes
Binary file not shown.

‎test/fixtures/es-modules/export-name-syntax-error.wat‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
;; Compiled using the WebAssembly Binary Toolkit (https://github.com/WebAssembly/wabt)
2+
;; $ wat2wasm export-name-syntax-error.wat
3+
4+
(module
5+
(global $0 i32 (i32.const 12682))
6+
(export "?f!o:o<b>a[r]" (global $0)))

‎test/fixtures/es-modules/import-name.wasm‎

237 Bytes
Binary file not shown.

‎test/fixtures/es-modules/import-name.wat‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
;; Compiled using the WebAssembly Binary Toolkit (https://github.com/WebAssembly/wabt)
2+
;; $ wat2wasm import-name.wat
3+
4+
(module
5+
(global $0 (import "./export-name-code-injection.wasm" ";import.meta.done=()=>{};console.log('code injection');{/*") i32)
6+
(global $1 (import "./export-name-code-injection.wasm" "/*/$;`//") i32)
7+
(global $2 (import "./export-name-syntax-error.wasm" "?f!o:o<b>a[r]") i32)
8+
(func $xor (result i32)
9+
(i32.xor (i32.xor (global.get $0) (global.get $1)) (global.get $2)))
10+
(export "xor" (func $xor)))

0 commit comments

Comments
 (0)