PayPro: Fix - verify every output against the signed invoice

packages/bitcore-wallet-client/src/lib/verifier.ts

@@ -305,34 +305,158 @@ export class Verifier {
     return true;
   }
 
-  static checkPaypro(txp, payproOpts) {
-    let toAddress, amount;
+  private static payproAddressesEqual(chain, address1, address2) {
+    if (typeof address1 !== 'string' || typeof address2 !== 'string') return false;
+    if (Constants.EVM_CHAINS.includes(chain)) {
+      return address1.toLowerCase() === address2.toLowerCase();
+    }
+    if (chain !== 'bch') return address1 === address2;
+    const normalize = address => {
+      try {
+        return new BCHAddress(address).toCashAddress(true);
+      } catch {
+        try {
+          return BCHAddress.fromObject(new Bitcore.Address(address).toObject()).toCashAddress(true);
+        } catch { return; }
+      }
+    };
+    const normalizedAddress = normalize(address1);
+    return !!normalizedAddress && normalizedAddress === normalize(address2);
+  }
 
-    if (parseInt(txp.version) >= 3) {
-      toAddress = txp.outputs[0].toAddress;
-      amount = txp.amount;
-    } else {
-      toAddress = txp.toAddress;
-      amount = txp.amount;
+  private static expectedPayproOutputs(chain, instructions): any[] | undefined {
+    if (!Array.isArray(instructions) || !instructions.length) return;
+
+    if (Constants.UTXO_CHAINS.includes(chain)) {
+      if (instructions.some(instruction => !Array.isArray(instruction?.outputs) || !instruction.outputs.length)) return;
+      return instructions.flatMap(instruction =>
+        instruction.outputs.map(output => ({
+          toAddress: output?.address,
+          amount: output?.amount
+        }))
+      );
+    }
+
+    if (Constants.EVM_CHAINS.includes(chain)) {
+      return instructions.map(instruction => ({
+        toAddress: instruction?.to,
+        amount: instruction?.value,
+        data: instruction?.data
+      }));
     }
 
-    if (amount != (payproOpts.instructions || []).reduce((sum, i) => sum += i.amount, 0)) return false;
+    if (Constants.RIPPLE_CHAINS.includes(chain) || Constants.SVM_CHAINS.includes(chain)) {
+      if (instructions.length !== 1) return;
+      const outputs = instructions[0]?.outputs;
+      if (!Array.isArray(outputs) || outputs.length !== 1) return;
+      const output = outputs[0];
+      return [{
+        toAddress: output?.address,
+        amount: output?.amount,
+        destinationTag: output?.destinationTag,
+        invoiceID: output?.invoiceID
+      }];
+    }
+  }
+
+  /**
+   * Verify every output against signed PayPro instructions so a copayer cannot hide unauthorized outputs.
+   */
+  static checkPaypro(txp, payproOpts) {
+    const chain = typeof txp?.chain === 'string'
+      ? txp.chain.toLowerCase()
+      : Utils.getChain(txp?.coin); // backwards compatibility
 
-    if (txp.coin == 'btc' && toAddress != payproOpts.instructions[0].toAddress)
+    if (!Constants.CHAINS.includes(chain)) {
+      log.debug(`[TXP ${txp?.id}] Unsupported PayPro chain`);
+      return false;
+    }
+    // EVM addresses are identical across chains and networks, so comparing the destination alone
+    // won't catch a proposal paying a mainnet address against a testnet (or other chain) invoice.
+    if (chain !== payproOpts?.chain?.toLowerCase()) {
+      log.debug(`[TXP ${txp?.id}] PayPro chain does not match transaction proposal`);
+      return false;
+    }
+    if (txp?.network !== payproOpts?.network) {
+      log.debug(`[TXP ${txp?.id}] PayPro network does not match transaction proposal`);
       return false;
+    }
+    if (!(parseInt(txp?.version) >= 3)) {
+      log.debug(`[TXP ${txp?.id}] Transaction proposal version not supported by PayPro`);
+      return false;
+    }
 
-    // Workaround for cashaddr/legacy address problems...
-    if (
-      txp.coin == 'bch' &&
-      new BCHAddress(toAddress).toString() !=
-        new BCHAddress(payproOpts.instructions[0].toAddress).toString()
-    )
+    const outputs = txp.outputs;
+    const expectedOutputs = this.expectedPayproOutputs(chain, payproOpts?.instructions);
+    if (!expectedOutputs?.length) {
+      log.debug(`[TXP ${txp?.id}] Invalid PayPro instructions`);
+      return false;
+    }
+    if (!Array.isArray(outputs)) {
+      log.debug(`[TXP ${txp?.id}] Invalid transaction proposal outputs`);
+      return false;
+    }
+    if (outputs.length !== expectedOutputs.length) {
+      log.debug(`[TXP ${txp?.id}] PayPro output count does not match transaction proposal`);
       return false;
+    }
+
+    for (let i = 0; i < outputs.length; i++) {
+      const output = outputs[i];
+      const expectedOutput = expectedOutputs[i];
+      if (!this.atomicValuesEqual(output?.amount, expectedOutput.amount)) {
+        log.debug(`[TXP ${txp?.id}] PayPro output ${i} amount does not match transaction proposal`);
+        return false;
+      }
+      if (!this.payproAddressesEqual(chain, output?.toAddress, expectedOutput.toAddress)) {
+        log.debug(`[TXP ${txp?.id}] PayPro output ${i} address does not match transaction proposal`);
+        return false;
+      }
+      if (
+        Constants.EVM_CHAINS.includes(chain) &&
+        (i === 0 && txp.data ? txp.data : output?.data) !== expectedOutput.data
+      ) {
+        log.debug(`[TXP ${txp?.id}] PayPro output ${i} data does not match transaction proposal`);
+        return false;
+      }
+    }
+
+    if (Constants.RIPPLE_CHAINS.includes(chain)) {
+      if (txp.multiTx) {
+        log.debug(`[TXP ${txp?.id}] PayPro does not support XRP multiTx transaction proposals`);
+        return false;
+      }
+      if (
+        txp.txType != null &&
+        (typeof txp.txType !== 'string' || txp.txType.toLowerCase() !== 'payment')
+      ) {
+        log.debug(`[TXP ${txp?.id}] PayPro does not support non-payment XRP transaction proposals`);
+        return false;
+      }
+      // XRP puts both the destination tag and the invoice ID on-chain, and the merchant reconciles
+      // the payment with them - so neither is free for the proposal to choose.
+      if (this.normalizeAtomicValue(expectedOutputs[0].destinationTag) === 0n) {
+        log.debug(`[TXP ${txp?.id}] PayPro destination tag 0 is not supported`);
+        return false;
+      }
+      if (!this.optionalAtomicValuesEqual(txp.destinationTag, expectedOutputs[0].destinationTag)) {
+        log.debug(`[TXP ${txp?.id}] PayPro destination tag does not match transaction proposal`);
+        return false;
+      }
+      if ((txp.invoiceID ?? null) !== (expectedOutputs[0].invoiceID ?? null)) {
+        log.debug(`[TXP ${txp?.id}] PayPro invoice ID does not match transaction proposal`);
+        return false;
+      }
+    }
 
-    // this generates problems...
-    //  if (feeRate && payproOpts.requiredFeeRate &&
-    //      feeRate < payproOpts.requiredFeeRate)
-    //  return false;
+    // On SOL the invoice ID travels as a memo instruction rather than as a transaction field
+    if (
+      Constants.SVM_CHAINS.includes(chain) &&
+      (txp.memo ?? null) !== (expectedOutputs[0].invoiceID ?? null)
+    ) {
+      log.debug(`[TXP ${txp?.id}] PayPro memo does not match transaction proposal`);
+      return false;
+    }
 
     return true;
   }

packages/bitcore-wallet-client/test/verifier.test.ts

@@ -188,6 +188,194 @@ describe('Verifier', function() {
     });
   });
 
+  describe('checkPaypro', function() {
+    const createPaypro = (chain, outputs) => ({
+      chain,
+      network: 'livenet',
+      instructions: [{ outputs }]
+    });
+    const createTxp = (paypro, outputs) => ({
+      version: 3,
+      coin: paypro.chain,
+      chain: paypro.chain,
+      network: paypro.network,
+      outputs
+    });
+    const addresses = {
+      btc: ['1LqBGSKuX5yYUonjxT5qGfpUsXKYYWeabA', '1BpEi6DfDAUFd7GtittLSdBeYJvcoaVggu'],
+      bch: ['qpm2qsznhks23z7629mms6s4cwef74vcwvy22gdx6a', 'qrvcdmgpk73zyfd8pmdl9wnuld36zh9n4gms8s0u59'],
+      doge: ['DH5yaieqoZN36fDVciNyRueRGvGLR3mr7L', 'DTdKu8YgcxoXyjFCDtCeKimaZzsK27rcwT'],
+      ltc: ['MTf4tP1TCNBn8dNkyxeBVoPrFCcVzxJvvh', 'MQMcJhpWHYVeQArcZR3sBgyPZxxRtnH441']
+    };
+
+    it('should verify every UTXO output address and amount', function() {
+      for (const [chain, [address, otherAddress]] of Object.entries(addresses)) {
+        const paypro = createPaypro(chain, [{ address, amount: 10000 }]);
+        const txp = createTxp(paypro, [{ toAddress: address, amount: 10000 }]);
+
+        Verifier.checkPaypro(txp, paypro).should.be.true;
+        Verifier.checkPaypro({
+          ...txp,
+          outputs: [{ toAddress: otherAddress, amount: 10000 }]
+        }, paypro).should.be.false;
+        Verifier.checkPaypro({
+          ...txp,
+          outputs: [{ toAddress: address, amount: 9999 }]
+        }, paypro).should.be.false;
+        Verifier.checkPaypro({
+          ...txp,
+          outputs: [
+            { toAddress: address, amount: 1 },
+            { toAddress: otherAddress, amount: 9999 }
+          ]
+        }, paypro).should.be.false;
+      }
+    });
+
+    it('should verify every output of UTXO instructions', function() {
+      const outputs = [
+        { address: addresses.btc[0], amount: 6000 },
+        { address: addresses.btc[1], amount: 4000 }
+      ];
+      const paypro = createPaypro('btc', outputs);
+      const txp = createTxp(paypro, outputs.map(output => ({
+        toAddress: output.address,
+        amount: output.amount
+      })));
+
+      Verifier.checkPaypro(txp, paypro).should.be.true;
+      Verifier.checkPaypro({ ...txp, outputs: [txp.outputs[0]] }, paypro).should.be.false;
+      Verifier.checkPaypro({ ...txp, outputs: [...txp.outputs].reverse() }, paypro).should.be.false;
+      Verifier.checkPaypro(txp, {
+        ...paypro,
+        instructions: [
+          { outputs: [outputs[0]] },
+          { outputs: [outputs[1]] }
+        ]
+      }).should.be.true;
+    });
+
+    it('should accept equivalent BCH encodings and reject malformed addresses', function() {
+      const paypro = createPaypro('bch', [{ address: addresses.bch[0], amount: 10000 }]);
+      const txp = createTxp(paypro, [{
+        toAddress: '1BpEi6DfDAUFd7GtittLSdBeYJvcoaVggu',
+        amount: 10000
+      }]);
+
+      Verifier.checkPaypro(txp, paypro).should.be.true;
+      Verifier.checkPaypro({
+        ...txp,
+        outputs: [{ toAddress: 'not-an-address', amount: 10000 }]
+      }, paypro).should.be.false;
+    });
+
+    it('should reject unsupported or mismatched chain, network, or instruction shape', function() {
+      const paypro = createPaypro('btc', [{ address: addresses.btc[0], amount: 10000 }]);
+      const txp = createTxp(paypro, [{ toAddress: addresses.btc[0], amount: 10000 }]);
+
+      Verifier.checkPaypro({ ...txp, chain: 'doge' }, paypro).should.be.false;
+      Verifier.checkPaypro({ ...txp, network: 'testnet' }, paypro).should.be.false;
+      Verifier.checkPaypro(txp, { ...paypro, instructions: [] }).should.be.false;
+      Verifier.checkPaypro({ ...txp, chain: 'unknown' }, { ...paypro, chain: 'unknown' }).should.be.false;
+    });
+
+    it('should bind EVM payments to their contract and calldata', function() {
+      const contract = '0xc27eD3DF0DE776246cdAD5a052A9982473FceaB8';
+      const paypro = {
+        chain: 'eth',
+        network: 'livenet',
+        instructions: [{ to: contract, value: 0, data: '0x095ea7b3aaaa' }]
+      };
+      const txp = createTxp(paypro, [{
+        toAddress: contract.toLowerCase(),
+        amount: 0,
+        data: '0x095ea7b3aaaa'
+      }]);
+
+      Verifier.checkPaypro(txp, paypro).should.be.true;
+      Verifier.checkPaypro({ ...txp, data: '0x095ea7b3deadbeef' }, paypro).should.be.false;
+      Verifier.checkPaypro({
+        ...txp,
+        outputs: [{ ...txp.outputs[0], toAddress: '0x0000000000000000000000000000000000000001' }]
+      }, paypro).should.be.false;
+    });
+
+    it('should bind XRP payments to the destination tag and invoice ID', function() {
+      const address = 'rEqj9WKSH7wEkPvWf6b4gCi26Y3F7HbKUF';
+      const invoiceID = '1012345678901234567890123456710123456789012345678901567890123456';
+      const paypro = {
+        chain: 'xrp',
+        network: 'livenet',
+        instructions: [{ outputs: [{ address, amount: 10000, destinationTag: 12345, invoiceID }] }]
+      };
+      const txp = {
+        ...createTxp(paypro, [{ toAddress: address, amount: 10000 }]),
+        destinationTag: '12345',
+        invoiceID
+      };
+      const zeroTagPaypro = {
+        ...paypro,
+        instructions: [{ outputs: [{ ...paypro.instructions[0].outputs[0], destinationTag: 0 }] }]
+      };
+
+      Verifier.checkPaypro(txp, paypro).should.be.true;
+      Verifier.checkPaypro({ ...txp, destinationTag: 999 }, paypro).should.be.false;
+      Verifier.checkPaypro({ ...txp, multiTx: true }, paypro).should.be.false;
+      Verifier.checkPaypro({ ...txp, txType: 'accountdelete' }, paypro).should.be.false;
+      // The XRP transaction builders currently omit a zero destination tag
+      Verifier.checkPaypro({ ...txp, destinationTag: '0' }, zeroTagPaypro).should.be.false;
+      // both reach the ledger and the merchant reconciles the payment with them
+      Verifier.checkPaypro({ ...txp, invoiceID: `2${invoiceID.slice(1)}` }, paypro).should.be.false;
+      Verifier.checkPaypro({ ...txp, invoiceID: undefined }, paypro).should.be.false;
+    });
+
+    it('should bind SOL payments to the invoice ID carried as a memo', function() {
+      const address = 'So11111111111111111111111111111111111111112';
+      const paypro = {
+        chain: 'sol',
+        network: 'livenet',
+        instructions: [{ outputs: [{ address, amount: 10000, invoiceID: 'LanynqCPoL2JQb8z8s5Z3X' }] }]
+      };
+      const txp = {
+        ...createTxp(paypro, [{ toAddress: address, amount: 10000 }]),
+        memo: 'LanynqCPoL2JQb8z8s5Z3X'
+      };
+
+      Verifier.checkPaypro(txp, paypro).should.be.true;
+      Verifier.checkPaypro({ ...txp, memo: 'GsbhMZeeUebqzEeDmNubEP' }, paypro).should.be.false;
+      Verifier.checkPaypro({ ...txp, memo: undefined }, paypro).should.be.false;
+    });
+
+    it('should bind every instruction of a multi-step EVM payment', function() {
+      // ERC20 invoices use separate approve and payment instructions
+      const tokenContract = '0xFEb423814D0208e9e2a3F5B0F0171e97376E20Bc';
+      const paymentContract = '0xc27eD3DF0DE776246cdAD5a052A9982473FceaB8';
+      const paypro = {
+        chain: 'eth',
+        network: 'livenet',
+        instructions: [
+          { to: tokenContract, value: 0, data: '0x095ea7b3aaaa' },
+          { to: paymentContract, value: 0, data: '0xd7bb99babbbb' }
+        ]
+      };
+      const outputs = paypro.instructions.map(i => ({ toAddress: i.to, amount: i.value, data: i.data }));
+      const txp = createTxp(paypro, outputs);
+
+      Verifier.checkPaypro(txp, paypro).should.be.true;
+      // tampering with the second step must not slip past a correct first one
+      Verifier.checkPaypro({
+        ...txp,
+        outputs: [outputs[0], { ...outputs[1], data: '0xd7bb99badeadbeef' }]
+      }, paypro).should.be.false;
+      Verifier.checkPaypro({
+        ...txp,
+        outputs: [outputs[0], { ...outputs[1], toAddress: tokenContract }]
+      }, paypro).should.be.false;
+      // dropping a step is not a valid payment either
+      Verifier.checkPaypro({ ...txp, outputs: [outputs[0]] }, paypro).should.be.false;
+    });
+  });
+
   describe('checkAddress', function() {
     it('should verify a BTC  address', () => {
       const cred = aKey.createCredentials(null, { coin: 'btc', network: 'livenet', account: 0, n: 1 });