[bisq-network/bisq] Fix BSQ swap buyer tx fee theft vulnerability (PR #5889)

[Steven Barclay]

<!-- 
- make yourself familiar with the CONTRIBUTING.md if you have not already (https://github.com/bisq-network/bisq/blob/master/CONTRIBUTING.md)
- make sure you follow our [coding style guidelines][https://github.com/bisq-network/style/issues)
- pick a descriptive title
- provide some meaningful PR description below
- create the PR
- in case you receive a "Change request" and/or a NACK, please react within 30 days. If not, we will close your PR and it can not be up for compensation.
- After addressing the change request, __please re-request a review!__ Otherwise we might miss your PR as we tend to only look at pull requests tagged with a "review required".
-->

Prevent the seller from stealing the combined tx fee as change by lying about the value of one or more of his BTC inputs, which are passed to the buyer as raw inputs in the `BsqSwapFinalizeTxRequest` message.

To this end, add a `RawTransactionInput::validate` method to check the `value` field against the output value of the respective spending tx and run it on every raw seller input in `ProcessBsqSwapFinalizeTxRequest`, so that the buyer is no longer just trusting those numbers.

Additionally, check that the spending txIds from the raw BTC inputs supplied by the seller actually match those of his signed inputs in the accompanying partially signed tx, thus tying the raw input values to the seller's tx.

--

This should block an attack by the BTC seller that I was able to carry out on regtest. If the seller is the taker, the attack can be made worse by creating a swap trade with the tx fee set at just under twice that seen by the maker, so that it is still within tolerance. So the seller-as-taker could wait until the tx fee spikes to, say, 50 sat/vbyte (but the mempool is not too full) and then take an offer with the tx fee set to 99 sat/vbyte. In this way, they could steal around 12000 sat or so as change and still leave 1 sat/vbyte for the fee so that the transaction eventually confirms. The attack is made worse the more inputs the maker uses or the higher the broadcast tx fee.

You can view, comment on, or merge this pull request online at:

  https://github.com/bisq-network/bisq/pull/5889

-- Commit Summary --

  * Fix misleading comment: BSQ swap taker tx fee tolerance
  * Fix BSQ swap buyer tx fee theft vulnerability
  * Validate input script types to prevent BSQ swap tx fee underpaying

-- File Changes --

    M core/src/main/java/bisq/core/btc/model/RawTransactionInput.java (26)
    M core/src/main/java/bisq/core/trade/bsq_swap/BsqSwapTakeOfferRequestVerification.java (8)
    M core/src/main/java/bisq/core/trade/protocol/bsq_swap/tasks/buyer/ProcessBsqSwapFinalizeTxRequest.java (19)
    M core/src/main/java/bisq/core/trade/protocol/bsq_swap/tasks/seller/ProcessTxInputsMessage.java (4)

-- Patch Links --

https://github.com/bisq-network/bisq/pull/5889.patch
https://github.com/bisq-network/bisq/pull/5889.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/5889
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20211202/bf05f9a0/attachment.htm>