fix(abstract-utxo): reject invalid derivation indexes

Author: Daan-25

modules/abstract-utxo/src/address/fixedScript.ts

@@ -72,7 +72,11 @@ export function generateAddressWithChainAndIndex(
  */
 export function generateAddress(coinName: UtxoCoinName, params: GenerateFixedScriptAddressOptions): string {
   let derivationIndex = 0;
-  if (_.isInteger(params.index) && (params.index as number) > 0) {
+  if (!_.isUndefined(params.index)) {
+    if (!_.isInteger(params.index) || (params.index as number) < 0) {
+      throw new InvalidAddressDerivationPropertyError(`address validation failure: invalid index (${params.index})`);
+    }
+
     derivationIndex = params.index as number;
   }
 
@@ -157,7 +161,12 @@ export function assertFixedScriptWalletAddress(
     address: string;
   }
 ): void {
-  if ((_.isUndefined(chain) && _.isUndefined(index)) || !(_.isFinite(chain) && _.isFinite(index))) {
+  if (
+    (_.isUndefined(chain) && _.isUndefined(index)) ||
+    !(_.isFinite(chain) && _.isFinite(index)) ||
+    !_.isInteger(index) ||
+    (index as number) < 0
+  ) {
     throw new InvalidAddressDerivationPropertyError(
       `address validation failure: invalid chain (${chain}) or index (${index})`
     );

modules/abstract-utxo/test/unit/address-index-edge.test.ts

@@ -0,0 +1,73 @@
+import * as assert from 'assert';
+
+import { InvalidAddressDerivationPropertyError } from '@bitgo/sdk-core';
+
+import { assertFixedScriptWalletAddress, generateAddress } from '../../src';
+
+const keychains = [
+  {
+    pub: 'xpub661MyMwAqRbcGiQhVk1J7cD1YodF9tc5Y1B8vpTjjB1pcB1J1m1QX8fMtYP2sYqFmW6J2ra69tNoARKjvTGo9cGUrbPbJdjwrSzGGzPzWWS',
+  },
+  {
+    pub: 'xpub661MyMwAqRbcFzLXuganogQvd7MrefQQqCcJP2ZDumnCdQecf5cw1P1nD5qBz8SNS1yCLSC9VqpNUWnQU3V6qmnPt2r21oXhicQFzPA6Lby',
+  },
+  {
+    pub: 'xpub661MyMwAqRbcFHpwWrzPB61U2CgBmdD21WNVM1JKUn9rEExkoGE4yafUVFbPSd78vdX8tWcEUQWaALFkU9fUbUM4Cc49DKEJSCYGRnbzCym',
+  },
+];
+
+describe('fixedScript address index edge cases', function () {
+  const chain = 20;
+
+  for (const invalidIndex of [-1, 1.5]) {
+    it(`rejects invalid derivation index ${invalidIndex}`, function () {
+      assert.throws(
+        () =>
+          generateAddress('btc', {
+            keychains,
+            chain,
+            index: invalidIndex,
+          }),
+        InvalidAddressDerivationPropertyError
+      );
+    });
+
+    it(`rejects index ${invalidIndex} during address validation`, function () {
+      const address0 = generateAddress('btc', {
+        keychains,
+        chain,
+        index: 0,
+      });
+
+      assert.throws(
+        () =>
+          assertFixedScriptWalletAddress('btc', {
+            chain,
+            index: invalidIndex,
+            keychains,
+            format: 'base58',
+            address: address0,
+          }),
+        InvalidAddressDerivationPropertyError
+      );
+    });
+  }
+
+  it('still accepts index 0', function () {
+    const address0 = generateAddress('btc', {
+      keychains,
+      chain,
+      index: 0,
+    });
+
+    assert.doesNotThrow(() =>
+      assertFixedScriptWalletAddress('btc', {
+        chain,
+        index: 0,
+        keychains,
+        format: 'base58',
+        address: address0,
+      })
+    );
+  });
+});