diff --git a/CVE-2023-38552.patch b/CVE-2023-38552.patch new file mode 100644 index 0000000000000000000000000000000000000000..1015d23f49595738cbfdaedab2fcd7ed4aba94ab --- /dev/null +++ b/CVE-2023-38552.patch @@ -0,0 +1,244 @@ +From 1c538938ccadfd35fbc699d8e85102736cd5945c Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= +Date: Sun, 6 Aug 2023 12:56:02 +0000 +Subject: [PATCH] policy: use tamper-proof integrity check function + +Using the JavaScript Hash class is unsafe because its internals can be +tampered with. In particular, an application can cause +Hash.prototype.digest() to return arbitrary values, thus allowing to +circumvent the integrity verification that policies are supposed to +guarantee. + +Add and use a new C++ binding internalVerifyIntegrity() that (hopefully) +cannot be tampered with from JavaScript. + +PR-URL: https://github.com/nodejs-private/node-private/pull/462 +Backport-PR-URL: https://github.com/nodejs-private/node-private/pull/493 +Reviewed-By: Rafael Gonzaga +CVE-ID: CVE-2023-38552 +--- + lib/internal/policy/manifest.js | 17 ++----- + src/crypto/crypto_hash.cc | 50 +++++++++++++++++++ + src/crypto/crypto_hash.h | 2 + + .../crypto-hash-tampering/.gitattributes | 1 + + .../policy/crypto-hash-tampering/main.js | 8 +++ + .../policy/crypto-hash-tampering/policy.json | 15 ++++++ + .../policy/crypto-hash-tampering/protected.js | 1 + + .../test-policy-crypto-hash-tampering.js | 21 ++++++++ + 8 files changed, 102 insertions(+), 13 deletions(-) + create mode 100644 test/fixtures/policy/crypto-hash-tampering/.gitattributes + create mode 100644 test/fixtures/policy/crypto-hash-tampering/main.js + create mode 100644 test/fixtures/policy/crypto-hash-tampering/policy.json + create mode 100644 test/fixtures/policy/crypto-hash-tampering/protected.js + create mode 100644 test/parallel/test-policy-crypto-hash-tampering.js + +diff --git a/lib/internal/policy/manifest.js b/lib/internal/policy/manifest.js +index 698971ee57f2b..d2afb10ec30e4 100644 +--- a/lib/internal/policy/manifest.js ++++ b/lib/internal/policy/manifest.js +@@ -16,7 +16,6 @@ const { + StringPrototypeEndsWith, + StringPrototypeStartsWith, + Symbol, +- uncurryThis, + } = primordials; + const { + ERR_MANIFEST_ASSERT_INTEGRITY, +@@ -28,13 +27,8 @@ let debug = require('internal/util/debuglog').debuglog('policy', (fn) => { + debug = fn; + }); + const SRI = require('internal/policy/sri'); +-const crypto = require('crypto'); +-const { Buffer } = require('buffer'); + const { URL } = require('internal/url'); +-const { createHash, timingSafeEqual } = crypto; +-const HashUpdate = uncurryThis(crypto.Hash.prototype.update); +-const HashDigest = uncurryThis(crypto.Hash.prototype.digest); +-const BufferToString = uncurryThis(Buffer.prototype.toString); ++const { internalVerifyIntegrity } = internalBinding('crypto'); + const kRelativeURLStringPattern = /^\.{0,2}\//; + const { getOptionValue } = require('internal/options'); + const shouldAbortOnUncaughtException = getOptionValue( +@@ -588,16 +582,13 @@ class Manifest { + // Avoid clobbered Symbol.iterator + for (let i = 0; i < integrityEntries.length; i++) { + const { algorithm, value: expected } = integrityEntries[i]; +- const hash = createHash(algorithm); + // TODO(tniessen): the content should not be passed as a string in the + // first place, see https://github.com/nodejs/node/issues/39707 +- HashUpdate(hash, content, 'utf8'); +- const digest = HashDigest(hash, 'buffer'); +- if (digest.length === expected.length && +- timingSafeEqual(digest, expected)) { ++ const mismatchedIntegrity = internalVerifyIntegrity(algorithm, content, expected); ++ if (mismatchedIntegrity === undefined) { + return true; + } +- realIntegrities.set(algorithm, BufferToString(digest, 'base64')); ++ realIntegrities.set(algorithm, mismatchedIntegrity); + } + } + +diff --git a/src/crypto/crypto_hash.cc b/src/crypto/crypto_hash.cc +index 200603a85ef33..3cb39d795c732 100644 +--- a/src/crypto/crypto_hash.cc ++++ b/src/crypto/crypto_hash.cc +@@ -69,6 +69,9 @@ void Hash::Initialize(Environment* env, Local target) { + SetMethodNoSideEffect(context, target, "getHashes", GetHashes); + + HashJob::Initialize(env, target); ++ ++ SetMethodNoSideEffect( ++ context, target, "internalVerifyIntegrity", InternalVerifyIntegrity); + } + + void Hash::RegisterExternalReferences(ExternalReferenceRegistry* registry) { +@@ -78,6 +81,8 @@ void Hash::RegisterExternalReferences(ExternalReferenceRegistry* registry) { + registry->Register(GetHashes); + + HashJob::RegisterExternalReferences(registry); ++ ++ registry->Register(InternalVerifyIntegrity); + } + + void Hash::New(const FunctionCallbackInfo& args) { +@@ -310,5 +315,50 @@ bool HashTraits::DeriveBits( + return true; + } + ++void InternalVerifyIntegrity(const v8::FunctionCallbackInfo& args) { ++ Environment* env = Environment::GetCurrent(args); ++ ++ CHECK_EQ(args.Length(), 3); ++ ++ CHECK(args[0]->IsString()); ++ Utf8Value algorithm(env->isolate(), args[0]); ++ ++ CHECK(args[1]->IsString() || IsAnyByteSource(args[1])); ++ ByteSource content = ByteSource::FromStringOrBuffer(env, args[1]); ++ ++ CHECK(args[2]->IsArrayBufferView()); ++ ArrayBufferOrViewContents expected(args[2]); ++ ++ const EVP_MD* md_type = EVP_get_digestbyname(*algorithm); ++ unsigned char digest[EVP_MAX_MD_SIZE]; ++ unsigned int digest_size; ++ if (md_type == nullptr || EVP_Digest(content.data(), ++ content.size(), ++ digest, ++ &digest_size, ++ md_type, ++ nullptr) != 1) { ++ return ThrowCryptoError( ++ env, ERR_get_error(), "Digest method not supported"); ++ } ++ ++ if (digest_size != expected.size() || ++ CRYPTO_memcmp(digest, expected.data(), digest_size) != 0) { ++ Local error; ++ MaybeLocal rc = ++ StringBytes::Encode(env->isolate(), ++ reinterpret_cast(digest), ++ digest_size, ++ BASE64, ++ &error); ++ if (rc.IsEmpty()) { ++ CHECK(!error.IsEmpty()); ++ env->isolate()->ThrowException(error); ++ return; ++ } ++ args.GetReturnValue().Set(rc.FromMaybe(Local())); ++ } ++} ++ + } // namespace crypto + } // namespace node +diff --git a/src/crypto/crypto_hash.h b/src/crypto/crypto_hash.h +index 96a9804420db6..2d17c3510ed21 100644 +--- a/src/crypto/crypto_hash.h ++++ b/src/crypto/crypto_hash.h +@@ -82,6 +82,8 @@ struct HashTraits final { + + using HashJob = DeriveBitsJob; + ++void InternalVerifyIntegrity(const v8::FunctionCallbackInfo& args); ++ + } // namespace crypto + } // namespace node + +diff --git a/test/fixtures/policy/crypto-hash-tampering/.gitattributes b/test/fixtures/policy/crypto-hash-tampering/.gitattributes +new file mode 100644 +index 0000000000000..cbdcbbc258e6e +--- /dev/null ++++ b/test/fixtures/policy/crypto-hash-tampering/.gitattributes +@@ -0,0 +1 @@ ++*.js text eol=lf +diff --git a/test/fixtures/policy/crypto-hash-tampering/main.js b/test/fixtures/policy/crypto-hash-tampering/main.js +new file mode 100644 +index 0000000000000..2ee233fe75461 +--- /dev/null ++++ b/test/fixtures/policy/crypto-hash-tampering/main.js +@@ -0,0 +1,8 @@ ++const h = require('crypto').createHash('sha384'); ++const fakeDigest = h.digest(); ++ ++const kHandle = Object.getOwnPropertySymbols(h) ++ .find((s) => s.description === 'kHandle'); ++h[kHandle].constructor.prototype.digest = () => fakeDigest; ++ ++require('./protected.js'); +diff --git a/test/fixtures/policy/crypto-hash-tampering/policy.json b/test/fixtures/policy/crypto-hash-tampering/policy.json +new file mode 100644 +index 0000000000000..3d981911533f5 +--- /dev/null ++++ b/test/fixtures/policy/crypto-hash-tampering/policy.json +@@ -0,0 +1,15 @@ ++{ ++ "resources": { ++ "./main.js": { ++ "integrity": true, ++ "dependencies": { ++ "./protected.js": true, ++ "crypto": true ++ } ++ }, ++ "./protected.js": { ++ "integrity": "sha384-OLBgp1GsljhM2TJ+sbHjaiH9txEUvgdDTAzHv2P24donTt6/529l+9Ua0vFImLlb", ++ "dependencies": true ++ } ++ } ++} +diff --git a/test/fixtures/policy/crypto-hash-tampering/protected.js b/test/fixtures/policy/crypto-hash-tampering/protected.js +new file mode 100644 +index 0000000000000..2b57adba5b191 +--- /dev/null ++++ b/test/fixtures/policy/crypto-hash-tampering/protected.js +@@ -0,0 +1 @@ ++console.log(require('fs').readFileSync('/etc/passwd').length); +diff --git a/test/parallel/test-policy-crypto-hash-tampering.js b/test/parallel/test-policy-crypto-hash-tampering.js +new file mode 100644 +index 0000000000000..96066defb59a1 +--- /dev/null ++++ b/test/parallel/test-policy-crypto-hash-tampering.js +@@ -0,0 +1,21 @@ ++'use strict'; ++ ++const common = require('../common'); ++if (!common.hasCrypto) ++ common.skip('missing crypto'); ++common.requireNoPackageJSONAbove(); ++ ++const fixtures = require('../common/fixtures'); ++ ++const assert = require('assert'); ++const { spawnSync } = require('child_process'); ++ ++const mainPath = fixtures.path('policy', 'crypto-hash-tampering', 'main.js'); ++const policyPath = fixtures.path( ++ 'policy', ++ 'crypto-hash-tampering', ++ 'policy.json'); ++const { status, stderr } = ++ spawnSync(process.execPath, ['--experimental-policy', policyPath, mainPath], { encoding: 'utf8' }); ++assert.strictEqual(status, 1); ++assert(stderr.includes('sha384-Bnp/T8gFNzT9mHj2G/AeuMH8LcAQ4mljw15nxBNl5yaGM7VgbMzDT7O4+dXZTJJn')); diff --git a/CVE-2023-39333.patch b/CVE-2023-39333.patch new file mode 100644 index 0000000000000000000000000000000000000000..f03c15ec5112ded833f6117d144a8798d752302d --- /dev/null +++ b/CVE-2023-39333.patch @@ -0,0 +1,161 @@ +From eaf9083cf1e43bd897ac8244dcc0f4e3500150ca 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/490 +Reviewed-By: Rafael Gonzaga +CVE-ID: CVE-2023-39333 +--- + .../modules/esm/create_dynamic_module.js | 14 +++--- + test/es-module/test-esm-wasm.mjs | 50 +++++++++++++++++++ + .../es-modules/export-name-code-injection.wat | 8 +++ + .../es-modules/export-name-syntax-error.wat | 6 +++ + test/fixtures/es-modules/import-name.wat | 10 ++++ + 5 files changed, 81 insertions(+), 7 deletions(-) + create mode 100644 test/fixtures/es-modules/export-name-code-injection.wat + create mode 100644 test/fixtures/es-modules/export-name-syntax-error.wat + 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 f7c20083..c99da19d 100644 +--- a/lib/internal/modules/esm/create_dynamic_module.js ++++ b/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, + };`; + } + +diff --git a/test/es-module/test-esm-wasm.mjs b/test/es-module/test-esm-wasm.mjs +index fac1d4b2..be33e3d4 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.wat b/test/fixtures/es-modules/export-name-code-injection.wat +new file mode 100644 +index 00000000..3cca749e +--- /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.wat b/test/fixtures/es-modules/export-name-syntax-error.wat +new file mode 100644 +index 00000000..fe8728e8 +--- /dev/null ++++ b/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))) +diff --git a/test/fixtures/es-modules/import-name.wat b/test/fixtures/es-modules/import-name.wat +new file mode 100644 +index 00000000..3501aa5e +--- /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))) +-- +2.30.0 + diff --git a/export-name-code-injection.wasm b/export-name-code-injection.wasm new file mode 100644 index 0000000000000000000000000000000000000000..c88b6b81e3f85230ab5cfe61a933c74c2bf05d01 Binary files /dev/null and b/export-name-code-injection.wasm differ diff --git a/export-name-syntax-error.wasm b/export-name-syntax-error.wasm new file mode 100644 index 0000000000000000000000000000000000000000..30787c5ab54ed27f4eeedbc29e33e031fdc44fd7 Binary files /dev/null and b/export-name-syntax-error.wasm differ diff --git a/import-name.wasm b/import-name.wasm new file mode 100644 index 0000000000000000000000000000000000000000..1c261b7a45af0fe53934388c1f246792b17cbf44 Binary files /dev/null and b/import-name.wasm differ diff --git a/nodejs.spec b/nodejs.spec index d8c9d7a27e60d6d6312605dc4769c9cdd9a7b861..1272c532f4f30a43206a1bf2652c45e066b0474c 100644 --- a/nodejs.spec +++ b/nodejs.spec @@ -1,4 +1,4 @@ -%global baserelease 1 +%global baserelease 2 %{?!_pkgdocdir:%global _pkgdocdir %{_docdir}/%{name}-%{version}} %global nodejs_epoch 1 %global nodejs_major 18 @@ -75,8 +75,18 @@ Source1: npmrc Source2: btest402.js Source3: https://github.com/unicode-org/icu/releases/download/release-%{icu_major}-%{icu_minor}/icu4c-%{icu_major}_%{icu_minor}-data-bin-l.zip Source4: nodejs_native.attr +# https://github.com/nodejs/node/blob/main/test/fixtures/es-modules/export-name-code-injection.wasm +Source5: export-name-code-injection.wasm +# https://github.com/nodejs/node/blob/main/test/fixtures/es-modules/export-name-syntax-error.wasm +Source6: export-name-syntax-error.wasm +# https://github.com/nodejs/node/blob/main/test/fixtures/es-modules/import-name.wasm +Source7: import-name.wasm Patch0: 0001-Use-system-uv-zlib.patch +# https://github.com/nodejs/node/commit/eaf9083cf1e43bd897ac8244dcc0f4e3500150ca +Patch1: CVE-2023-39333.patch +# https://github.com/nodejs/node/commit/1c538938ccadfd35fbc699d8e85102736cd5945c +Patch2: CVE-2023-38552.patch BuildRequires: python3-devel python3-setuptools make BuildRequires: zlib-devel python3-jinja2 @@ -215,6 +225,10 @@ sed -i "s~usr\/bin\/python.*$~usr\/bin\/python3~" ./deps/v8/tools/mb/mb_test.py find . -type f -exec sed -i "s~python -c~python3 -c~" {} \; %build +cp %{SOURCE5} test/fixtures/es-modules/ +cp %{SOURCE6} test/fixtures/es-modules/ +cp %{SOURCE7} test/fixtures/es-modules/ + %global optflags %(echo %{optflags} | sed 's/-g /-g1 /') export CC='%{__cc}' @@ -406,6 +420,9 @@ NODE_PATH=%{buildroot}%{_prefix}/lib/node_modules:%{buildroot}%{_prefix}/lib/nod %{_pkgdocdir}/npm/docs %changelog +* Mon Dec 25 2023 yaoxin - 1:18.17.1-2 +- Fix CVE-2023-39333 and CVE-2023-38552 + * Thu Aug 31 2023 Funda Wang - 1:18.17.1-1 - Update to 18.17.1 - Use huaweicloud.com mirror as default registry