[BWS][BWC][CWC] Bind prePublishRaw fallback to the proposal

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

@@ -1,7 +1,8 @@
 import {
   BitcoreLib as Bitcore,
   BitcoreLibCash,
-  Utils as CWCUtils
+  Utils as CWCUtils,
+  Transactions
 } from '@bitpay-labs/crypto-wallet-core';
 import { singleton } from 'preconditions';
 import { Constants, Utils } from './common';
@@ -278,14 +279,24 @@ export class Verifier {
     log.debug(`[TXP ${txp.id}] Regenerating & verifying tx proposal hash -> Hash: ${hash}, Signature: ${txp.proposalSignature}`);
 
     const verified = Utils.verifyMessage(hash, txp.proposalSignature, creatorSigningPubKey);
-    if (!verified && !txp.prePublishRaw) {
-      log.debug(`[TXP ${txp.id}] Invalid proposal signature, no prePublishRaw to fall back to`);
-      return false;
-    }
-
-    if (!verified && txp.prePublishRaw && !Utils.verifyMessage(txp.prePublishRaw, txp.proposalSignature, creatorSigningPubKey)) {
-      log.debug(`[TXP ${txp.id}] Invalid proposal signature, even with prePublishRaw fallback`);
-      return false;
+    if (!verified) {
+      // Local rebuild != creator's signature. Legit only when BWS mutated a field at publish (SVM recent
+      // blockhash, or EVM/XRP deferred nonce): the creator signed the pre-publish serialization, stored as
+      // txp.prePublishRaw. Fall back to it only if the signature is valid over it AND it is bound to this
+      // proposal -- else a hostile server could pair a valid (prePublishRaw, proposalSignature) with a
+      // tampered destination. See checkPrePublishRaw.
+      if (!txp.prePublishRaw) {
+        log.debug(`[TXP ${txp.id}] Invalid proposal signature, no prePublishRaw to fall back to`);
+        return false;
+      }
+      if (!Utils.verifyMessage(txp.prePublishRaw, txp.proposalSignature, creatorSigningPubKey)) {
+        log.debug(`[TXP ${txp.id}] Invalid proposal signature, even with prePublishRaw fallback`);
+        return false;
+      }
+      if (!this.checkPrePublishRaw(chain, txp)) {
+        log.warn(`[TXP ${txp.id}] prePublishRaw is not bound to this proposal; possible server tampering`);
+        return false;
+      }
     }
 
     if (Constants.UTXO_CHAINS.includes(chain)) {
@@ -305,6 +316,51 @@ export class Verifier {
     return true;
   }
 
+  /**
+   * True only if txp.prePublishRaw is the same transaction as the current proposal, differing solely in a
+   * field BWS mutates at publish (SVM blockhash / EVM-XRP nonce). Binds the creator's fallback signature to
+   * this proposal: without it a compromised server could pair a valid (prePublishRaw, proposalSignature)
+   * with a tampered destination/amount. Rejects non-mutable chains and fails closed on any error.
+   *
+   * @param {string} chain - lower-cased chain of the proposal
+   * @param {Object} txp - the transaction proposal (must carry prePublishRaw)
+   */
+  static checkPrePublishRaw(chain, txp) {
+    // Only chains with a publish-mutable serialized field legitimately carry prePublishRaw: the recent
+    // blockhash (SVM) or account nonce (EVM/XRP). Anywhere else (e.g. UTXO) its presence is illegitimate.
+    const canHaveMutableLifetime = [
+      ...Constants.SVM_CHAINS,
+      ...Constants.EVM_CHAINS,
+      ...Constants.RIPPLE_CHAINS
+    ].includes(chain);
+    if (!canHaveMutableLifetime) {
+      log.warn(`[TXP ${txp.id}] prePublishRaw present on chain ${chain} that cannot mutate at publish; refusing fallback`);
+      return false;
+    }
+    try {
+      const prePublishRaw = Array.isArray(txp.prePublishRaw) ? txp.prePublishRaw : [txp.prePublishRaw];
+      // Recover the mutable field (blockhash / nonce) from the pre-publish serialization the creator signed;
+      // the stored proposal carries the refreshed value, so it isn't readable off txp directly.
+      const provider: any = Transactions.get({ chain });
+      const mutableFields = provider.getMutableFields(prePublishRaw[0]);
+      if (!mutableFields || Object.values(mutableFields).every(v => v == null)) {
+        log.warn(`[TXP ${txp.id}] Could not recover pre-publish mutable fields from prePublishRaw; refusing fallback`);
+        return false;
+      }
+      // Rebuild with the pre-publish mutable field: an untampered proposal reproduces prePublishRaw exactly;
+      // any changed field (destination, amount, from, contract) serializes differently and fails the compare.
+      const rebuilt = Utils.buildTx({ ...txp, ...mutableFields }).uncheckedSerialize();
+      const rebuiltArr = Array.isArray(rebuilt) ? rebuilt : [rebuilt];
+      if (rebuiltArr.length !== prePublishRaw.length) {
+        return false;
+      }
+      return rebuiltArr.every((raw, i) => raw === prePublishRaw[i]);
+    } catch (err) {
+      log.warn(`[TXP ${txp.id}] Failed to verify prePublishRaw binding: ${err?.message || err}`);
+      return false;
+    }
+  }
+
   static checkPaypro(txp, payproOpts) {
     let toAddress, amount;
 

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

@@ -1,6 +1,7 @@
 'use strict';
 
 import chai from 'chai';
+import { Utils } from '../src/lib/common';
 import { Verifier } from '../src/lib/verifier';
 import { Key } from '../src/lib/key';
 
@@ -222,4 +223,61 @@ describe('Verifier', function() {
       }).should.be.true;
     });
   });
+
+  describe('checkPrePublishRaw', function() {
+    const ATTACKER_EVM = '0x1111111111111111111111111111111111111111';
+    const ATTACKER_SOL = 'F7FknkRckx4yvA3Gexnx1H3nwPxndMxVt58BwAzEQhcY';
+
+    const solTxp = (overrides = {}) => ({
+      chain: 'sol',
+      category: 'transfer',
+      from: '8WyoNvKsmfdG6zrbzNBVN8DETyLra3ond61saU9C52YR',
+      outputs: [{ toAddress: '3xkNjKm2zvGRvH2z2Nf9Y5hFvHZFtQXbQb1PBxDT56Xy', amount: 3896000000000000 }],
+      blockHash: 'GtV1Hb3FvP3HURHAsj8mGwEqCumvP3pv3i6CVCzYNj3d',
+      blockHeight: 531575,
+      ...overrides
+    });
+
+    const evmTxp = (overrides = {}) => ({
+      chain: 'eth',
+      network: 'livenet',
+      outputs: [{ toAddress: '0x37d7B3bBD88EFdE6a93cF74D2F5b0385D3E3B08A', amount: 1000 }],
+      from: '0x37d7B3bBD88EFdE6a93cF74D2F5b0385D3E3B08A',
+      nonce: 0,
+      gasLimit: 21000,
+      gasPrice: 20000000000,
+      data: '0x',
+      ...overrides
+    });
+
+    it('accepts a SOL proposal whose blockhash was refreshed at publish', function() {
+      const prePublishRaw = Utils.buildTx(solTxp()).uncheckedSerialize();
+      const current = solTxp({ blockHash: 'H1oRr1nfr4b6eZjs9Ssn3bxUmcRAjqRxrbTKuwSPZ9mE', prePublishRaw });
+      Verifier.checkPrePublishRaw('sol', current).should.be.true;
+    });
+
+    it('rejects a SOL proposal whose destination was tampered', function() {
+      const prePublishRaw = Utils.buildTx(solTxp()).uncheckedSerialize();
+      const current = solTxp({
+        blockHash: 'H1oRr1nfr4b6eZjs9Ssn3bxUmcRAjqRxrbTKuwSPZ9mE',
+        outputs: [{ toAddress: ATTACKER_SOL, amount: 3896000000000000 }],
+        prePublishRaw
+      });
+      Verifier.checkPrePublishRaw('sol', current).should.be.false;
+    });
+
+    it('rejects an EVM proposal whose destination was tampered', function() {
+      const prePublishRaw = Utils.buildTx(evmTxp()).uncheckedSerialize();
+      const current = evmTxp({
+        nonce: 9,
+        outputs: [{ toAddress: ATTACKER_EVM, amount: 1000 }],
+        prePublishRaw
+      });
+      Verifier.checkPrePublishRaw('eth', current).should.be.false;
+    });
+
+    it('rejects prePublishRaw on a non-mutable (UTXO) chain', function() {
+      Verifier.checkPrePublishRaw('btc', { chain: 'btc', prePublishRaw: 'anything' }).should.be.false;
+    });
+  });
 });

packages/bitcore-wallet-service/src/lib/chain/btc/index.ts

@@ -349,6 +349,11 @@ export class BtcChain implements IChain {
     });
   }
 
+  isPrePublishRawBound(_txp: TxProposal) {
+    // UTXO chains never mutate tx data at publish, so they never legitimately carry prePublishRaw.
+    return false;
+  }
+
   getBitcoreTx(txp, opts = { signed: true }) {
     const t = new this.bitcoreLib.Transaction();
 

packages/bitcore-wallet-service/src/lib/chain/eth/index.ts

@@ -287,6 +287,10 @@ export class EthChain implements IChain {
     });
   }
 
+  isPrePublishRawBound(txp: TxProposal) {
+    return Common.Utils.isPrePublishRawBound(this, txp);
+  }
+
   getBitcoreTx(txp: TxProposal, opts = { signed: true }) {
     const {
       data,

packages/bitcore-wallet-service/src/lib/chain/index.ts

@@ -98,6 +98,10 @@ class ChainProxy {
     return this.get(txp.chain).getBitcoreTx(txp, { signed: opts.signed });
   }
 
+  isPrePublishRawBound(txp: TxProposal<any>) {
+    return this.get(txp.chain).isPrePublishRawBound(txp);
+  }
+
   convertFeePerKb(chain, p, feePerKb) {
     return this.get(chain).convertFeePerKb(p, feePerKb);
   }

packages/bitcore-wallet-service/src/lib/chain/sol/index.ts

@@ -2,6 +2,7 @@ import { Transactions, Utils, Validation } from '@bitpay-labs/crypto-wallet-core
 import _ from 'lodash';
 import { IChain } from '../../../types/chain';
 import { WalletWithOpts } from '../../blockchainexplorers/v8';
+import { Common } from '../../common';
 import { Defaults } from '../../common/defaults';
 import { Errors } from '../../errors/errordefinitions';
 import logger from '../../logger';
@@ -98,6 +99,10 @@ export class SolChain implements IChain {
     });
   }
 
+  isPrePublishRawBound(txp) {
+    return Common.Utils.isPrePublishRawBound(this, txp);
+  }
+
   getBitcoreTx(txp, opts = { signed: true }) {
     const {
       data,

packages/bitcore-wallet-service/src/lib/chain/xrp/index.ts

@@ -143,6 +143,10 @@ export class XrpChain implements IChain {
     });
   }
 
+  isPrePublishRawBound(txp) {
+    return Common.Utils.isPrePublishRawBound(this, txp);
+  }
+
   getBitcoreTx(txp, opts = { signed: true }) {
     const { destinationTag, outputs, outputOrder, multiTx } = txp;
     const chain = 'XRP';

packages/bitcore-wallet-service/src/lib/common/utils.ts

@@ -3,7 +3,8 @@ import {
   BitcoreLibCash,
   BitcoreLibDoge,
   BitcoreLibLtc,
-  Constants as CWConstants
+  Constants as CWConstants,
+  Transactions
 } from '@bitpay-labs/crypto-wallet-core';
 import _ from 'lodash';
 import { singleton } from 'preconditions';
@@ -23,6 +24,34 @@ const Bitcore_ = {
 
 export const Utils = {
 
+  /**
+   * Shared implementation of IChain.isPrePublishRawBound for account-based chains (SVM/EVM/XRP). True only if
+   * txp.prePublishRaw is the same transaction as the current proposal, differing solely in the field BWS
+   * mutates at publish (blockhash on SVM, nonce on EVM/XRP). Recovers that field from prePublishRaw, rebuilds
+   * the proposal via the chain's own getBitcoreTx, and requires byte-for-byte equality -- so a tampered
+   * stored proposal cannot reuse an old, still-valid proposalSignature. Fails closed on any error.
+   *
+   * @param chain - the IChain implementation (provides chain + getBitcoreTx)
+   * @param txp - the transaction proposal (must carry prePublishRaw)
+   */
+  isPrePublishRawBound(chain, txp): boolean {
+    try {
+      const prePublishRaw = Array.isArray(txp.prePublishRaw) ? txp.prePublishRaw : [txp.prePublishRaw];
+      const provider = Transactions.get({ chain: txp.chain }) as any;
+      if (typeof provider?.getMutableFields !== 'function') return false;
+      const mutableFields = provider.getMutableFields(prePublishRaw[0]);
+      if (!mutableFields || Object.values(mutableFields).every(v => v == null)) return false;
+      const cloned = Object.assign(Object.create(Object.getPrototypeOf(txp)), txp, mutableFields);
+      const rebuilt = chain.getBitcoreTx(cloned).uncheckedSerialize();
+      const rebuiltArr = Array.isArray(rebuilt) ? rebuilt : [rebuilt];
+      if (rebuiltArr.length !== prePublishRaw.length) return false;
+      return rebuiltArr.every((raw, i) => raw === prePublishRaw[i]);
+    } catch (err) {
+      logger.warn('prePublishRaw binding check failed: %o', err);
+      return false;
+    }
+  },
+
   /**
    * @deprecated
    * Moved from ChainService to prevent circular dependency

packages/bitcore-wallet-service/src/lib/server.ts

@@ -3063,8 +3063,12 @@ export class WalletService implements IWalletService {
 
           let signingKey = this._getSigningKey(raw, opts.proposalSignature, copayer.requestPubKeys);
           if (!signingKey) {
-            // If the txp has been published previously, we will verify the signature against the previously published raw tx
-            if (txp.hasMutableTxData() && txp.prePublishRaw) {
+            // The signature didn't match the current (refreshed) raw tx. Fall back to the previously-published
+            // raw tx -- but only if it is bound to THIS proposal (differs from the current tx solely in the
+            // field BWS mutates at publish: blockhash/nonce). Without that check a tampered stored proposal
+            // could reuse an old, still-valid proposalSignature over prePublishRaw. Mirrors the client
+            // Verifier.checkPrePublishRaw guard.
+            if (txp.hasMutableTxData() && txp.prePublishRaw && ChainService.isPrePublishRawBound(txp)) {
               raw = txp.prePublishRaw;
               signingKey = this._getSigningKey(raw, opts.proposalSignature, copayer.requestPubKeys);
             }

packages/bitcore-wallet-service/src/types/chain.d.ts

@@ -33,6 +33,7 @@ export interface IChain {
   checkScriptOutput(output: { script: string; amount: number });
   getFee(server: WalletService, wallet: IWallet, opts: { fee: number; feePerKb: number; signatures?: number } & any);
   getBitcoreTx(txp: TxProposal, opts: { signed: boolean });
+  isPrePublishRawBound(txp: TxProposal): boolean;
   convertFeePerKb(p: number, feePerKb: number);
   checkTx(server: WalletService, txp: ITxProposal);
   checkTxUTXOs(server: WalletService, txp: ITxProposal, opts: { noCashAddr: boolean } & any, cb);

packages/bitcore-wallet-service/test/integration/server.test.ts

@@ -8319,6 +8319,64 @@ describe('Wallet service', function() {
       result.nonce.should.equal(5);
       // future: result.gasPrice, result.maxFee would also be set here
     });
+
+    describe('prePublishRaw binding (isPrePublishRawBound)', function() {
+      const ATTACKER_ADDR = '0x1111111111111111111111111111111111111111';
+
+      it('accepts prePublishRaw when only the mutable field (nonce) changed', async function() {
+        blockchainExplorer.getTransactionCount = sinon.stub().callsArgWith(1, null, '42');
+        const created = await helpers.createAndPublishTx(server, {
+          outputs: [{ toAddress: ETH_ADDR, amount: 8000 }],
+          feePerKb: 123e2, from: fromAddr, deferNonce: true
+        }, TestData.copayers[0].privKey_1H_0);
+
+        // Server assigns the JIT nonce -> the stored tx now differs from prePublishRaw only in the nonce.
+        await util.promisify(server.prepareTx).call(server, { txProposalId: created.id });
+        const withNonce = await util.promisify(server.getTx).call(server, { txProposalId: created.id });
+        should.exist(withNonce.prePublishRaw);
+        withNonce.nonce.should.equal(42);
+
+        ChainService.isPrePublishRawBound(withNonce).should.equal(true);
+      });
+
+      it('rejects prePublishRaw when the destination was tampered', async function() {
+        const created = await helpers.createAndPublishTx(server, {
+          outputs: [{ toAddress: ETH_ADDR, amount: 8000 }],
+          feePerKb: 123e2, from: fromAddr, deferNonce: true
+        }, TestData.copayers[0].privKey_1H_0);
+
+        const txp = await util.promisify(server.getTx).call(server, { txProposalId: created.id });
+        should.exist(txp.prePublishRaw);
+        ChainService.isPrePublishRawBound(txp).should.equal(true); // bound before tampering
+        txp.outputs[0].toAddress = ATTACKER_ADDR; // swap payee, keep prePublishRaw + proposalSignature
+        ChainService.isPrePublishRawBound(txp).should.equal(false);
+      });
+
+      it('rejects the publishTx fallback when the stored proposal was tampered', async function() {
+        blockchainExplorer.getTransactionCount = sinon.stub().callsArgWith(1, null, '7');
+        const created = await util.promisify(server.createTx).call(server, {
+          outputs: [{ toAddress: ETH_ADDR, amount: 8000 }],
+          feePerKb: 123e2, from: fromAddr, deferNonce: true
+        });
+        const publishOpts = helpers.getProposalSignatureOpts(created, TestData.copayers[0].privKey_1H_0);
+        await util.promisify(server.publishTx).call(server, publishOpts);
+
+        // Assign the nonce so a re-publish must fall back to prePublishRaw...
+        await util.promisify(server.prepareTx).call(server, { txProposalId: created.id });
+        // ...then tamper the stored destination and replay the original (still-valid) proposalSignature.
+        const stored = await util.promisify(server.storage.fetchTx).call(server.storage, wallet.id, created.id);
+        should.exist(stored.prePublishRaw);
+        stored.outputs[0].toAddress = ATTACKER_ADDR;
+        await util.promisify(server.storage.storeTx).call(server.storage, wallet.id, stored);
+
+        let err;
+        try {
+          await util.promisify(server.publishTx).call(server, publishOpts);
+        } catch (e) { err = e; }
+        should.exist(err);
+        err.message.should.contain('Invalid proposal signature');
+      });
+    });
   });
 
   describe('#prepareTx XRP (deferred nonce)', function() {
@@ -8410,6 +8468,20 @@ describe('Wallet service', function() {
       });
       signed.status.should.equal('accepted');
     });
+
+    it('rejects prePublishRaw binding when an XRP destination was tampered', async function() {
+      const ATTACKER_XRP = 'rPT1Sjq2YGrBMTttX4GZHjKu9dyfzbpAYe';
+      const created = await helpers.createAndPublishTx(server, {
+        outputs: [{ toAddress: XRP_ADDR, amount: 8000 }],
+        feePerKb: 123e2, from: fromAddr, deferNonce: true
+      }, TestData.copayers[0].privKey_1H_0);
+
+      const txp = await util.promisify(server.getTx).call(server, { txProposalId: created.id });
+      should.exist(txp.prePublishRaw);
+      ChainService.isPrePublishRawBound(txp).should.equal(true); // untampered baseline
+      txp.outputs[0].toAddress = ATTACKER_XRP;
+      ChainService.isPrePublishRawBound(txp).should.equal(false);
+    });
   });
 
   describe('#signTx nonce override', function() {

packages/crypto-wallet-core/src/transactions/eth/index.ts

@@ -127,6 +127,15 @@ export class ETHTxProvider {
     return ethers.Transaction.from(tx).hash;
   }
 
+  /**
+   * Reads the fields BWS may mutate between proposal creation and publish out of a raw tx, to bind
+   * txp.prePublishRaw to the proposal. For EVM that is only the account nonce (deferred-nonce proposals).
+   */
+  getMutableFields(rawTx: string): { nonce?: number } {
+    const parsed = ethers.Transaction.from(rawTx);
+    return { nonce: parsed.nonce != null ? Number(parsed.nonce) : undefined };
+  }
+
   applySignature(params: { tx: string; signature: any }) {
     const { tx, signature } = params;
     const parsedTx = ethers.Transaction.from(tx);