[bisq-network/bisq] [WIP] Fix remaining blackmail vulnerabilities (#4789)

Steven Barclay notifications at github.com
Fri Nov 13 09:49:00 CET 2020


<!-- 
- 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".
-->

This PR attempts to take advantage of the SegWit trade protocol upgrade (hard fork), to fix all the remaining cases where a buyer or seller can publish a deposit tx without their peer having a valid delayed payout tx (at least in the fully SegWit case, when taking an offer created in v1.5.0+). When taking pre-1.5.0 offers, the deposit tx inputs will be of mixed type, which makes it malleable and thus impossible to completely prevent either trader from altering its ID and thus rendering their peer's delayed payout tx invalid. In that case, the buyer should still fully validate their delayed payout tx anyway, against the agreed deposit tx, so that if the latter changes before confirming, the trade hopefully fails before the buyer makes payment. The PR additionally fixes a bug where (it appears) the buyer fails to check the signature of the final delayed payout tx.

These changes are intended to prevent blackmail attacks (or reduce the risk of them in not-fully-SegWit cases where the deposit tx is malleable).

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

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

-- Commit Summary --

  * Fix missing segwit case when sanitising preparedDepositTx
  * Send seller's delayedPayoutTx signature to peer ASAP
  * Add new BuyerFinalizesDelayedPayoutTx task
  * Withhold witnesses in buyer->seller depositTx data, until last step

-- File Changes --

    M core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java (56)
    M core/src/main/java/bisq/core/trade/messages/DelayedPayoutTxSignatureRequest.java (21)
    M core/src/main/java/bisq/core/trade/messages/DelayedPayoutTxSignatureResponse.java (27)
    M core/src/main/java/bisq/core/trade/messages/DepositTxMessage.java (16)
    M core/src/main/java/bisq/core/trade/protocol/BuyerAsMakerProtocol.java (2)
    M core/src/main/java/bisq/core/trade/protocol/BuyerAsTakerProtocol.java (2)
    M core/src/main/java/bisq/core/trade/protocol/SellerAsMakerProtocol.java (2)
    M core/src/main/java/bisq/core/trade/protocol/SellerAsTakerProtocol.java (2)
    M core/src/main/java/bisq/core/trade/protocol/SellerProtocol.java (2)
    A core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerFinalizesDelayedPayoutTx.java (60)
    M core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerProcessDelayedPayoutTxSignatureRequest.java (1)
    M core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerProcessDepositTxAndDelayedPayoutTxMessage.java (10)
    M core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerSendsDelayedPayoutTxSignatureResponse.java (7)
    M core/src/main/java/bisq/core/trade/protocol/tasks/buyer/BuyerVerifiesPreparedDelayedPayoutTx.java (23)
    M core/src/main/java/bisq/core/trade/protocol/tasks/buyer_as_maker/BuyerAsMakerSendsInputsForDepositTxResponse.java (7)
    M core/src/main/java/bisq/core/trade/protocol/tasks/buyer_as_taker/BuyerAsTakerSendsDepositTxMessage.java (4)
    M core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerFinalizesDelayedPayoutTx.java (3)
    M core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerProcessDelayedPayoutTxSignatureResponse.java (7)
    M core/src/main/java/bisq/core/trade/protocol/tasks/seller/SellerSendDelayedPayoutTxSignatureRequest.java (5)
    M core/src/main/java/bisq/core/trade/protocol/tasks/seller_as_maker/SellerAsMakerProcessDepositTxMessage.java (2)
    M core/src/main/java/bisq/core/trade/protocol/tasks/seller_as_maker/SellerAsMakerSendsInputsForDepositTxResponse.java (3)
    M proto/src/main/proto/pb.proto (6)

-- Patch Links --

https://github.com/bisq-network/bisq/pull/4789.patch
https://github.com/bisq-network/bisq/pull/4789.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/4789
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20201113/aca17230/attachment.html>


More information about the bisq-github mailing list