From 3b23b2ee531310e38a4ed7b659ef3e2d75e1e65d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Sun, 6 Aug 2023 10:41:33 +0000 Subject: [PATCH] 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: https://github.com/nodejs-private/node-private/pull/461 Backport-PR-URL: https://github.com/nodejs-private/node-private/pull/489 Reviewed-By: Rafael Gonzaga CVE-ID: CVE-2023-39333 --- .../modules/esm/create_dynamic_module.js | 15 +++--- test/es-module/test-esm-wasm.mjs | 50 ++++++++++++++++++ .../export-name-code-injection.wasm | Bin 0 -> 98 bytes .../es-modules/export-name-code-injection.wat | 8 +++ .../es-modules/export-name-syntax-error.wasm | Bin 0 -> 37 bytes .../es-modules/export-name-syntax-error.wat | 6 +++ test/fixtures/es-modules/import-name.wasm | Bin 0 -> 237 bytes test/fixtures/es-modules/import-name.wat | 10 ++++ 8 files changed, 82 insertions(+), 7 deletions(-) create mode 100644 test/fixtures/es-modules/export-name-code-injection.wasm create mode 100644 test/fixtures/es-modules/export-name-code-injection.wat create mode 100644 test/fixtures/es-modules/export-name-syntax-error.wasm create mode 100644 test/fixtures/es-modules/export-name-syntax-error.wat create mode 100644 test/fixtures/es-modules/import-name.wasm create mode 100644 test/fixtures/es-modules/import-name.wat diff --git a/lib/internal/modules/esm/create_dynamic_module.js b/lib/internal/modules/esm/create_dynamic_module.js index 2eac81a8221..d4f5a85db95 100644 --- a/lib/internal/modules/esm/create_dynamic_module.js +++ b/lib/internal/modules/esm/create_dynamic_module.js @@ -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, };`; } diff --git a/test/es-module/test-esm-wasm.mjs b/test/es-module/test-esm-wasm.mjs index fac1d4b2837..be33e3d437d 100644 --- a/test/es-module/test-esm-wasm.mjs +++ b/test/es-module/test-esm-wasm.mjs @@ -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:oa[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', diff --git a/test/fixtures/es-modules/export-name-code-injection.wasm b/test/fixtures/es-modules/export-name-code-injection.wasm new file mode 100644 index 0000000000000000000000000000000000000000..c88b6b81e3f85230ab5cfe61a933c74c2bf05d01 GIT binary patch literal 98 zcmZQbEY4+QU|?Y5WvXXz{LR1(Bu_ANvwJdGS!d=JZRo8rP^v}+S*mu zS|{h{73b%q>gD98Yp5sZr=%)m=4GWOmt^MWscTwS>uWJHaOi94t5_%K>oYR~05#zo AJ^%m! literal 0 HcmV?d00001 diff --git a/test/fixtures/es-modules/export-name-code-injection.wat b/test/fixtures/es-modules/export-name-code-injection.wat new file mode 100644 index 00000000000..3cca749ea3a --- /dev/null +++ b/test/fixtures/es-modules/export-name-code-injection.wat @@ -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))) diff --git a/test/fixtures/es-modules/export-name-syntax-error.wasm b/test/fixtures/es-modules/export-name-syntax-error.wasm new file mode 100644 index 0000000000000000000000000000000000000000..30787c5ab54ed27f4eeedbc29e33e031fdc44fd7 GIT binary patch literal 37 scmZQbEY4+QU|?Y5V610w?0U?=%`V8uYoDf=Za[r]" (global $0))) diff --git a/test/fixtures/es-modules/import-name.wasm b/test/fixtures/es-modules/import-name.wasm new file mode 100644 index 0000000000000000000000000000000000000000..1c261b7a45af0fe53934388c1f246792b17cbf44 GIT binary patch literal 237 zcmajZQ3`@U6ouh?l`yQmccPX!r2{f6PO^N)DV4>-leuxvNIBk<^;>r)9Q1Eh(nWF%vtzAAoHk UI1bt^X#jVME_mRf@hmfW0i2FT{Qv*} literal 0 HcmV?d00001 diff --git a/test/fixtures/es-modules/import-name.wat b/test/fixtures/es-modules/import-name.wat new file mode 100644 index 00000000000..3501aa5ead9 --- /dev/null +++ b/test/fixtures/es-modules/import-name.wat @@ -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:oa[r]") i32) + (func $xor (result i32) + (i32.xor (i32.xor (global.get $0) (global.get $1)) (global.get $2))) + (export "xor" (func $xor)))