fix: mitigate CVE-2024-48930 by validating secp256k1 curve points, enforce secp256k1 v5.0.1

TICKET: DX-1569

Author: starfy84

modules/bitgo/test/ecdh.ts

@@ -5,6 +5,7 @@ import 'should';
 import { bip32 } from '@bitgo/utxo-lib';
 import * as crypto from 'crypto';
 import * as utxolib from '@bitgo/utxo-lib';
+import * as secp256k1 from 'secp256k1';
 
 import { getSharedSecret, signMessageWithDerivedEcdhKey, verifyEcdhSignature } from '@bitgo/sdk-core';
 import { TestBitGo } from '@bitgo/sdk-test';
@@ -82,4 +83,26 @@ describe('ECDH utils', () => {
     isVerify = verifyEcdhSignature(JSON.stringify(message), signedMessage.toString('hex'), derivedPubKey);
     isVerify.should.be.false();
   });
+
+  it('getSharedSecret rejects invalid public keys for CVE-2024-48930 mitigation', function () {
+    const validPrivateKey = Buffer.alloc(32, 1); // Create a dummy private key
+    const validPublicKey = Buffer.from(secp256k1.publicKeyCreate(validPrivateKey));
+
+    // Valid key should work
+    const sharedSecret = getSharedSecret(validPrivateKey, validPublicKey);
+    assert.strictEqual(sharedSecret.length, 32);
+
+    // Test with invalid public key
+    const invalidPublicKey = Buffer.from('0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798', 'hex');
+    // Modify last byte to make it invalid but keep the format valid
+    invalidPublicKey[32] = invalidPublicKey[32] ^ 0xff;
+
+    try {
+      getSharedSecret(validPrivateKey, invalidPublicKey);
+      assert.fail('Should have thrown error for invalid public key');
+    } catch (e) {
+      // The test passes as long as an error is thrown
+      assert.ok(true, 'Error was thrown as expected');
+    }
+  });
 });

modules/express/encryptedPrivKeys.json

@@ -1,3 +1,3 @@
 {
-   "61f039aad587c2000745c687373e0fa9": "{\"iv\":\"O74H8BBv86GBpoTzjVyzWw==\",\"v\":1,\"iter\":10000,\"ks\":256,\"ts\":64,\"mode\":\"ccm\",\"adata\":\"\",\"cipher\":\"aes\",\"salt\":\"7n8pAjXCfug=\",\"ct\":\"14MjiKBksaaayrwuc/w8vJ5C3yflQ15//dhLiOgYVqjhJJ7iKrcrjtgfLoI3+MKLaKCycNKi6vTs2xs8xJeSm/XhsOE9EfapkfGHdYuf4C6O1whNOyugZ0ZSOA/buDC3rvBbvCNtLDOxN5XWJN/RADOnZdHuVGk=\"}"
+	"61f039aad587c2000745c687373e0fa9": "{\"iv\":\"v0AKnjfRjwYGs6fLm4nPIg==\",\"v\":1,\"iter\":10000,\"ks\":256,\"ts\":64,\"mode\":\"ccm\",\"adata\":\"\",\"cipher\":\"aes\",\"salt\":\"LiHJlj8SDlQ=\",\"ct\":\"yIfdY5ufDAPgfMpXPPPMlusy1MyAmBX5YsBcm4rO8wGsjBFsvvhJHdwaOTXEhwkg+HkZFzoQw2RxDx6ghfKYs/x6SnbY7lSALt9Q/pCYa+80p4/WfsS4OKpdfJ/lbtj9eDSTe8wMvV5Cc/b65qx7rAitFkdF4aw=\"}"
 }

modules/sdk-core/src/bitgo/ecdh.ts

@@ -16,6 +16,21 @@ import * as assert from 'assert';
 import * as secp256k1 from 'secp256k1';
 import { ECPairInterface, BIP32Interface, bip32, ECPair } from '@bitgo/utxo-lib';
 
+/**
+ * Validate that a public key is on the secp256k1 curve
+ * This is a defense-in-depth check to prevent invalid curve attacks
+ * @param publicKey Buffer containing the public key to validate
+ * @returns boolean true if the key is valid, false otherwise
+ */
+function validatePublicKey(publicKey: Buffer): boolean {
+  try {
+    // Validates that the public key is a point on the secp256k1 curve
+    return secp256k1.publicKeyVerify(publicKey);
+  } catch (e) {
+    return false;
+  }
+}
+
 /**
  * Calculate the Elliptic Curve Diffie Hellman
  * @param privateKey HDNode of private key
@@ -55,8 +70,15 @@ export function getSharedSecret(
   assert.strictEqual(privateKey.length, 32);
   assert.strictEqual(publicKey.length, 33);
 
+  // Extra validation to ensure the public key is on the curve
+  // This provides defense-in-depth against CVE-2024-48930 (GHSA-584q-6j8j-r5pm)
+  if (!validatePublicKey(publicKey)) {
+    throw new Error(`invalid public key: not on secp256k1 curve`);
+  }
+
   // FIXME(BG-34386): we should use `secp256k1.ecdh()` in the future
   //                  see discussion here https://github.com/bitcoin-core/secp256k1/issues/352
+  //                  Additional validation has been added above to protect against CVE-2024-48930
   const buffer = Buffer.from(secp256k1.publicKeyTweakMul(publicKey, privateKey))
     // remove leading parity bit
     .slice(1);

package.json

@@ -83,6 +83,11 @@
     "json5": "^2.2.2",
     "ua-parser-js": ">0.7.30 <0.8.0",
     "secp256k1": "5.0.1",
+    "**/ethereum-cryptography/secp256k1": "5.0.1",
+    "**/hdkey/secp256k1": "5.0.1",
+    "**/eccrypto/secp256k1": "5.0.1",
+    "**/bitcoinjs-message/secp256k1": "5.0.1",
+    "**/bip322-js/*/bitcoinjs-message/secp256k1": "5.0.1",
     "socks": "2.7.3",
     "web3-utils": "4.2.1",
     "@polkadot/api": "14.1.1",
@@ -142,5 +147,21 @@
     "terser": "^5.14.2",
     "tmp": "^0.2.3"
   },
+  "overrides": {
+    "ethereum-cryptography": {
+      "secp256k1": "5.0.1"
+    },
+    "hdkey": {
+      "secp256k1": "5.0.1"
+    },
+    "eccrypto": {
+      "secp256k1": "5.0.1"
+    },
+    "bip322-js": {
+      "bitcoinjs-message": {
+        "secp256k1": "5.0.1"
+      }
+    }
+  },
   "packageManager": "[email protected]"
 }

scripts/check-package-dependencies.ts

@@ -34,6 +34,7 @@ const options = {
     'sass-loader',
     'style-loader',
     'ts-loader',
+    'secp256k1',
   ],
   specials: [
     depcheck.special.babel,