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,