Merge pull request #7960 from BitGo/create-address-issue/WP-7507

fix: address validation for SMC wallets

Author: Marzooqa

.iyarc

@@ -4,4 +4,10 @@
 # - This CVE affects archive EXTRACTION (unpacking malicious symlinks/hardlinks)
 # - Lerna only uses tar for PACKING
 GHSA-8qq5-rm4j-mr97
+
+# Excluded because:
+# - Transitive dependency through lerna and yeoman-generator, which currently pin tar to a
+#   < 7.5.4 range; We only use their tar integration for
+#   archive PACKING, not extraction,
 GHSA-r6q2-hw4h-h46w
+

modules/sdk-coin-sol/test/unit/sol.ts

@@ -3478,6 +3478,21 @@ describe('SOL:', function () {
         message: `invalid address: ${invalidAddress}`,
       });
     });
+
+    it('should verify address with derivation prefix for SMC wallets', async function () {
+      // Address derived with derivation prefix m/999999/148242355/239336845/1
+      const address = 'CC715Q92C8vuEFT5xujE55Do6BkWRdpDvr7Vh8iUNUBw';
+      const commonKeychain =
+        '8ea32ecacfc83effbd2e2790ee44fa7c59b4d86c29a12f09fb613d8195f93f4e21875cad3b98adada40c040c54c3569467df41a020881a6184096378701862bd';
+      const index = '1';
+      const derivedFromParentWithSeed = 'smc-test-seed-123';
+      const keychains = [{ id: '1', type: 'tss' as const, commonKeychain }];
+
+      // This test verifies that derivedFromParentWithSeed is accepted as a parameter
+      // and the verification function correctly computes the derivation prefix internally
+      const result = await basecoin.isWalletAddress({ keychains, address, index, derivedFromParentWithSeed });
+      result.should.equal(true);
+    });
   });
 
   describe('getAddressFromPublicKey', () => {

modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts

@@ -154,6 +154,11 @@ export interface VerifyAddressOptions {
   error?: string;
   coinSpecific?: AddressCoinSpecific;
   impliedForwarderVersion?: number;
+  /**
+   * Optional seed value from user keychain's derivedFromParentWithSeed field.
+   * For SMC (Self-Managed Custodial) TSS wallets, this is used to compute the derivation prefix.
+   */
+  derivedFromParentWithSeed?: string;
 }
 
 /**
@@ -173,8 +178,15 @@ export interface TssVerifyAddressOptions {
   /**
    * Derivation index for the address.
    * Used to derive child addresses from the root keychain via HD derivation path: m/{index}
+   * For SMC (Self-Managed Custodial) wallets, the path includes a prefix: m/{derivationPrefix}/{index}
    */
   index: number | string;
+  /**
+   * Optional seed value from user keychain's derivedFromParentWithSeed field.
+   * For SMC (Self-Managed Custodial) wallets, this is used to compute the derivation prefix.
+   * The derivation path becomes {computedPrefix}/{index} instead of m/{index}.
+   */
+  derivedFromParentWithSeed?: string;
 }
 
 export function isTssVerifyAddressOptions<T extends VerifyAddressOptions | TssVerifyAddressOptions>(

modules/sdk-core/src/bitgo/utils/tss/addressVerification.ts

@@ -1,3 +1,4 @@
+import { getDerivationPath } from '@bitgo/sdk-lib-mpc';
 import { Ecdsa } from '../../../account-lib/mpc';
 import { TssVerifyAddressOptions } from '../../baseCoin/iBaseCoin';
 import { InvalidAddressError } from '../../errors';
@@ -65,15 +66,21 @@ export async function verifyMPCWalletAddress(
   isValidAddress: (address: string) => boolean,
   getAddressFromPublicKey: (publicKey: string) => string
 ): Promise<boolean> {
-  const { keychains, address, index } = params;
+  const { keychains, address, index, derivedFromParentWithSeed } = params;
 
   if (!isValidAddress(address)) {
     throw new InvalidAddressError(`invalid address: ${address}`);
   }
 
   const MPC = params.keyCurve === 'secp256k1' ? new Ecdsa() : await EDDSAMethods.getInitializedMpcInstance();
   const commonKeychain = extractCommonKeychain(keychains);
-  const derivedPublicKey = MPC.deriveUnhardened(commonKeychain, 'm/' + index);
+
+  // Compute derivation path:
+  // - For SMC wallets with derivedFromParentWithSeed, compute prefix and use: {prefix}/{index}
+  // - For other wallets, use simple path: m/{index}
+  const prefix = derivedFromParentWithSeed ? getDerivationPath(derivedFromParentWithSeed.toString()) : undefined;
+  const derivationPath = prefix ? `${prefix}/${index}` : `m/${index}`;
+  const derivedPublicKey = MPC.deriveUnhardened(commonKeychain, derivationPath);
 
   // secp256k1 expects 33 bytes; ed25519 expects 32 bytes
   const publicKeySize = params.keyCurve === 'secp256k1' ? 33 : 32;

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

@@ -32,7 +32,7 @@ import {
 import { SubmitTransactionResponse } from '../inscriptionBuilder';
 import { drawKeycard } from '../internal';
 import * as internal from '../internal/internal';
-import { decryptKeychainPrivateKey, Keychain, KeychainWithEncryptedPrv } from '../keychain';
+import { decryptKeychainPrivateKey, Keychain, KeychainWithEncryptedPrv, KeyIndices } from '../keychain';
 import { getLightningAuthKey } from '../lightning/lightningWalletUtil';
 import { IPendingApproval, PendingApproval, PendingApprovals } from '../pendingApproval';
 import { GoStakingWallet, StakingWallet } from '../staking';
@@ -1408,6 +1408,15 @@ export class Wallet implements IWallet {
       }
 
       verificationData.impliedForwarderVersion = forwarderVersion ?? verificationData.coinSpecific?.forwarderVersion;
+
+      // For SMC (Self-Managed Custodial) wallets, pass derivedFromParentWithSeed from user keychain
+      // The verification function will compute the derivation prefix internally
+      // Custodial wallets don't need this as their commonKeychain already accounts for the prefix
+      if (this.multisigType() === 'tss' && this.type() === 'cold' && this._wallet.keys.length > KeyIndices.USER) {
+        const userKeychain = keychains[KeyIndices.USER] as Keychain | undefined;
+        verificationData.derivedFromParentWithSeed = userKeychain?.derivedFromParentWithSeed;
+      }
+
       // This condition was added in first place because in celo, when verifyAddress method was called on addresses which were having pendingChainInitialization as true, it used to throw some error
       // In case of forwarder version 1 eth addresses, addresses need to be verified even if the pendingChainInitialization flag is true
       if (

modules/sdk-core/test/unit/bitgo/utils/tss/addressVerification.ts

@@ -0,0 +1,63 @@
+import * as assert from 'assert';
+import 'should';
+import { getDerivationPath } from '@bitgo/sdk-lib-mpc';
+
+function getAddressVerificationModule() {
+  return require('../../../../../src/bitgo/utils/tss/addressVerification');
+}
+
+const getExtractCommonKeychain = () => getAddressVerificationModule().extractCommonKeychain;
+
+describe('TSS Address Verification - Derivation Path with Prefix', function () {
+  const commonKeychain =
+    '8ea32ecacfc83effbd2e2790ee44fa7c59b4d86c29a12f09fb613d8195f93f4e21875cad3b98adada40c040c54c3569467df41a020881a6184096378701862bd';
+
+  describe('extractCommonKeychain', function () {
+    it('should extract commonKeychain from keychains array', function () {
+      const extractCommonKeychain = getExtractCommonKeychain();
+      const keychains = [{ commonKeychain }, { commonKeychain }, { commonKeychain }];
+
+      const result = extractCommonKeychain(keychains);
+      result.should.equal(commonKeychain);
+    });
+
+    it('should throw error if keychains are missing', function () {
+      const extractCommonKeychain = getExtractCommonKeychain();
+      assert.throws(() => extractCommonKeychain([]), /missing required param keychains/);
+      assert.throws(() => extractCommonKeychain(undefined as any), /missing required param keychains/);
+    });
+
+    it('should throw error if commonKeychain is missing', function () {
+      const extractCommonKeychain = getExtractCommonKeychain();
+      const keychains = [{ id: '1' }];
+      assert.throws(() => extractCommonKeychain(keychains as any), /missing required param commonKeychain/);
+    });
+
+    it('should throw error if keychains have mismatched commonKeychains', function () {
+      const extractCommonKeychain = getExtractCommonKeychain();
+      const keychains = [{ commonKeychain }, { commonKeychain: 'different-keychain' }];
+      assert.throws(() => extractCommonKeychain(keychains), /all keychains must have the same commonKeychain/);
+    });
+  });
+
+  describe('Derivation Path Format Validation', function () {
+    it('should produce correct derivation path format', function () {
+      const testSeeds = ['seed1', 'seed-2', '12345', 'test_seed_123'];
+
+      testSeeds.forEach((seed) => {
+        const path = getDerivationPath(seed);
+
+        // Expected format: "m/999999/{part1}/{part2}"
+        path.should.match(/^m\/999999\/\d+\/\d+$/);
+
+        path.should.startWith('m/');
+
+        // Should have exactly 3 parts
+        const parts = path.split('/');
+        parts.length.should.equal(4);
+        parts[0].should.equal('m');
+        parts[1].should.equal('999999');
+      });
+    });
+  });
+});