Skip to content

Commit

Permalink
module: fix code injection through export names
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tniessen authored and RafaelGSS committed Oct 13, 2023
1 parent e673c03 commit 3b23b2e
Show file tree
Hide file tree
Showing 8 changed files with 82 additions and 7 deletions.
15 changes: 8 additions & 7 deletions lib/internal/modules/esm/create_dynamic_module.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,15 @@ import.meta.imports[${imptPath}] = $import_${index};`;
/**
* Creates an export for a given module.
* @param {string} expt - The name of the export.
* @param {number} index - The index of the export statement.
*/
function createExport(expt) {
const name = `${expt}`;
return `let $${name};
export { $${name} as ${name} };
import.meta.exports.${name} = {
get: () => $${name},
set: (v) => $${name} = v,
function createExport(expt, index) {
const nameStringLit = JSONStringify(expt);
return `let $export_${index};
export { $export_${index} as ${nameStringLit} };
import.meta.exports[${nameStringLit}] = {
get: () => $export_${index},
set: (v) => $export_${index} = v,
};`;
}

Expand Down
50 changes: 50 additions & 0 deletions test/es-module/test-esm-wasm.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,56 @@ describe('ESM: WASM modules', { concurrency: true }, () => {
strictEqual(code, 0);
});

it('should not allow code injection through export names', async () => {
const { code, stderr, stdout } = await spawnPromisified(execPath, [
'--no-warnings',
'--experimental-wasm-modules',
'--input-type=module',
'--eval',
`import * as wasmExports from ${JSON.stringify(fixtures.fileURL('es-modules/export-name-code-injection.wasm'))};`,
]);

strictEqual(stderr, '');
strictEqual(stdout, '');
strictEqual(code, 0);
});

it('should allow non-identifier export names', async () => {
const { code, stderr, stdout } = await spawnPromisified(execPath, [
'--no-warnings',
'--experimental-wasm-modules',
'--input-type=module',
'--eval',
[
'import { strictEqual } from "node:assert";',
`import * as wasmExports from ${JSON.stringify(fixtures.fileURL('es-modules/export-name-syntax-error.wasm'))};`,
'assert.strictEqual(wasmExports["?f!o:o<b>a[r]"]?.value, 12682);',
].join('\n'),
]);

strictEqual(stderr, '');
strictEqual(stdout, '');
strictEqual(code, 0);
});

it('should properly escape import names as well', async () => {
const { code, stderr, stdout } = await spawnPromisified(execPath, [
'--no-warnings',
'--experimental-wasm-modules',
'--input-type=module',
'--eval',
[
'import { strictEqual } from "node:assert";',
`import * as wasmExports from ${JSON.stringify(fixtures.fileURL('es-modules/import-name.wasm'))};`,
'assert.strictEqual(wasmExports.xor(), 12345);',
].join('\n'),
]);

strictEqual(stderr, '');
strictEqual(stdout, '');
strictEqual(code, 0);
});

it('should emit experimental warning', async () => {
const { code, signal, stderr } = await spawnPromisified(execPath, [
'--experimental-wasm-modules',
Expand Down
Binary file not shown.
8 changes: 8 additions & 0 deletions test/fixtures/es-modules/export-name-code-injection.wat
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
;; Compiled using the WebAssembly Binary Toolkit (https://github.com/WebAssembly/wabt)
;; $ wat2wasm export-name-code-injection.wat

(module
(global $0 i32 (i32.const 123))
(global $1 i32 (i32.const 456))
(export ";import.meta.done=()=>{};console.log('code injection');{/*" (global $0))
(export "/*/$;`//" (global $1)))
Binary file not shown.
6 changes: 6 additions & 0 deletions test/fixtures/es-modules/export-name-syntax-error.wat
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
;; Compiled using the WebAssembly Binary Toolkit (https://github.com/WebAssembly/wabt)
;; $ wat2wasm export-name-syntax-error.wat

(module
(global $0 i32 (i32.const 12682))
(export "?f!o:o<b>a[r]" (global $0)))
Binary file added test/fixtures/es-modules/import-name.wasm
Binary file not shown.
10 changes: 10 additions & 0 deletions test/fixtures/es-modules/import-name.wat
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
;; Compiled using the WebAssembly Binary Toolkit (https://github.com/WebAssembly/wabt)
;; $ wat2wasm import-name.wat

(module
(global $0 (import "./export-name-code-injection.wasm" ";import.meta.done=()=>{};console.log('code injection');{/*") i32)
(global $1 (import "./export-name-code-injection.wasm" "/*/$;`//") i32)
(global $2 (import "./export-name-syntax-error.wasm" "?f!o:o<b>a[r]") i32)
(func $xor (result i32)
(i32.xor (i32.xor (global.get $0) (global.get $1)) (global.get $2)))
(export "xor" (func $xor)))

0 comments on commit 3b23b2e

Please sign in to comment.