Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
devel:languages:nodejs
nodejs12
CVE-2023-38552.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File CVE-2023-38552.patch of Package nodejs12
commit 1c538938ccadfd35fbc699d8e85102736cd5945c Author: Tobias Nießen <tniessen@tnie.de> Date: Sun Aug 6 12:56:02 2023 +0000 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 <rafael.nunu@hotmail.com> CVE-ID: CVE-2023-38552 Index: node-v12.22.12/lib/internal/policy/manifest.js =================================================================== --- node-v12.22.12.orig/lib/internal/policy/manifest.js +++ node-v12.22.12/lib/internal/policy/manifest.js @@ -26,13 +26,8 @@ let debug = require('internal/util/debug 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 = @@ -235,17 +230,14 @@ class Manifest { algorithm, value: expected } = integrityEntries[i]; - const hash = createHash(algorithm); - HashUpdate(hash, content); - const digest = HashDigest(hash); - if (digest.length === expected.length && - timingSafeEqual(digest, expected)) { + const mismatchedIntegrity = internalVerifyIntegrity(algorithm, content, expected); + if (mismatchedIntegrity === undefined) { return true; } MapPrototypeSet( realIntegrities, algorithm, - BufferToString(digest, 'base64') + mismatchedIntegrity ); } } Index: node-v12.22.12/test/fixtures/policy/crypto-hash-tampering/.gitattributes =================================================================== --- /dev/null +++ node-v12.22.12/test/fixtures/policy/crypto-hash-tampering/.gitattributes @@ -0,0 +1 @@ +*.js text eol=lf Index: node-v12.22.12/test/fixtures/policy/crypto-hash-tampering/main.js =================================================================== --- /dev/null +++ node-v12.22.12/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'); Index: node-v12.22.12/test/fixtures/policy/crypto-hash-tampering/policy.json =================================================================== --- /dev/null +++ node-v12.22.12/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 + } + } +} Index: node-v12.22.12/test/fixtures/policy/crypto-hash-tampering/protected.js =================================================================== --- /dev/null +++ node-v12.22.12/test/fixtures/policy/crypto-hash-tampering/protected.js @@ -0,0 +1 @@ +console.log(require('fs').readFileSync('/etc/passwd').length); Index: node-v12.22.12/test/parallel/test-policy-crypto-hash-tampering.js =================================================================== --- /dev/null +++ node-v12.22.12/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')); Index: node-v12.22.12/src/node_crypto.cc =================================================================== --- node-v12.22.12.orig/src/node_crypto.cc +++ node-v12.22.12/src/node_crypto.cc @@ -6655,6 +6655,53 @@ void GetCurves(const FunctionCallbackInf } +void InternalVerifyIntegrity(const FunctionCallbackInfo<Value>& 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()); + ArrayBufferViewContents<unsigned char> expected(args[2]); + //ArrayBufferOrViewContents<unsigned char> 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.get(), + content.size(), + digest, + &digest_size, + md_type, + nullptr) != 1) { + return ThrowCryptoError( + env, ERR_get_error(), "Digest method not supported"); + } + + if (digest_size != expected.length() || + CRYPTO_memcmp(digest, expected.data(), digest_size) != 0) { + Local<Value> error; + MaybeLocal<Value> rc = + StringBytes::Encode(env->isolate(), + reinterpret_cast<const char*>(digest), + digest_size, + BASE64, + &error); + if (rc.IsEmpty()) { + CHECK(!error.IsEmpty()); + env->isolate()->ThrowException(error); + return; + } + args.GetReturnValue().Set(rc.FromMaybe(Local<Value>())); + } +} + + bool VerifySpkac(const char* data, unsigned int len) { NetscapeSPKIPointer spki(NETSCAPE_SPKI_b64_decode(data, len)); if (!spki) @@ -7045,6 +7092,7 @@ void Initialize(Local<Object> target, env->SetMethodNoSideEffect(target, "getCiphers", GetCiphers); env->SetMethodNoSideEffect(target, "getHashes", GetHashes); env->SetMethodNoSideEffect(target, "getCurves", GetCurves); + env->SetMethodNoSideEffect(target, "internalVerifyIntegrity", InternalVerifyIntegrity); env->SetMethod(target, "publicEncrypt", PublicKeyCipher::Cipher<PublicKeyCipher::kPublic, EVP_PKEY_encrypt_init,
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
