commit eaf9083cf1e43bd897ac8244dcc0f4e3500150ca Author: Tobias Nießen Date: Sun Aug 6 10:41:33 2023 +0000 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/490 Reviewed-By: Rafael Gonzaga CVE-ID: CVE-2023-39333 commit 1193ca5fdb6e3eaa56b57a1b62cd48c7309b5d01 Author: Matthew Aitken Date: Fri Oct 6 17:04:05 2023 -0400 lib: let deps require `node` prefixed modules fixup fixup PR-URL: https://github.com/nodejs/node/pull/50047 Reviewed-By: Antoine du Hamel Reviewed-By: Filip Skokan Reviewed-By: Michaël Zasso Reviewed-By: Benjamin Gruenbaum Reviewed-By: Luigi Pinca Reviewed-By: Stephen Belanger Reviewed-By: Zeyu "Alex" Yang Includes wasm-fixtures.tar.gz because quilt doesn't support binary patches.... Index: node-v16.20.2/lib/internal/modules/esm/create_dynamic_module.js =================================================================== --- node-v16.20.2.orig/lib/internal/modules/esm/create_dynamic_module.js +++ node-v16.20.2/lib/internal/modules/esm/create_dynamic_module.js @@ -18,13 +18,13 @@ function createImport(impt, index) { import.meta.imports[${imptPath}] = $import_${index};`; } -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, };`; } Index: node-v16.20.2/test/es-module/test-esm-wasm.mjs =================================================================== --- node-v16.20.2.orig/test/es-module/test-esm-wasm.mjs +++ node-v16.20.2/test/es-module/test-esm-wasm.mjs @@ -29,6 +29,56 @@ describe('ESM: WASM modules', { concurre 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', Index: node-v16.20.2/test/fixtures/es-modules/export-name-code-injection.wat =================================================================== --- /dev/null +++ node-v16.20.2/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))) Index: node-v16.20.2/test/fixtures/es-modules/export-name-syntax-error.wat =================================================================== --- /dev/null +++ node-v16.20.2/test/fixtures/es-modules/export-name-syntax-error.wat @@ -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:oa[r]" (global $0))) Index: node-v16.20.2/test/fixtures/es-modules/import-name.wat =================================================================== --- /dev/null +++ node-v16.20.2/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))) Index: node-v16.20.2/lib/internal/bootstrap/loaders.js =================================================================== --- node-v16.20.2.orig/lib/internal/bootstrap/loaders.js +++ node-v16.20.2/lib/internal/bootstrap/loaders.js @@ -58,6 +58,7 @@ const { SafeSet, String, StringPrototypeStartsWith, + StringPrototypeSlice, TypeError, } = primordials; @@ -365,7 +366,9 @@ function nativeModuleRequire(id) { // Allow internal modules from dependencies to require // other modules from dependencies by providing fallbacks. function requireWithFallbackInDeps(request) { - if (!BuiltinModule.map.has(request)) { + if (StringPrototypeStartsWith(request, 'node:')) { + request = StringPrototypeSlice(request, 5); + } else if (!BuiltinModule.map.has(request)) { request = `internal/deps/${request}`; } return nativeModuleRequire(request);