From 63db7b3d35d43c4840a1ca35cc9df0a088e5146b Mon Sep 17 00:00:00 2001 From: Peter Date: Fri, 17 Nov 2023 17:04:45 +0100 Subject: [PATCH] fix: include scriptPubKey size in hash that is checked --- src/swap/Btc_Marketplace.sol | 32 ++++++++++++++++++++++++-------- test/swap/Btc_Marketplace.t.sol | 22 ++++++++++------------ 2 files changed, 34 insertions(+), 20 deletions(-) diff --git a/src/swap/Btc_Marketplace.sol b/src/swap/Btc_Marketplace.sol index 70f3ae73..0bb13d8e 100644 --- a/src/swap/Btc_Marketplace.sol +++ b/src/swap/Btc_Marketplace.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.13; import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; +import {BTCUtils} from "@bob-collective/bitcoin-spv/BTCUtils.sol"; import {BitcoinTx} from "../bridge/BitcoinTx.sol"; // import {LightRelay} from "../relay/LightRelay.sol"; import {IRelay} from "../bridge/IRelay.sol"; @@ -156,10 +157,7 @@ contract BtcMarketPlace { relay.validateProof(transaction, proof); - // Check output script pubkey (recipient address) and amount - uint256 txOutputValue = - BitcoinTx.getTxOutputValue(keccak256(accept.bitcoinAddress.scriptPubKey), transaction.outputVector); - assert(txOutputValue >= accept.amountBtc); + _checkBitcoinTxOutput(accept.amountBtc, accept.bitcoinAddress, transaction); IERC20(accept.ercToken).safeTransfer(accept.requester, accept.ercAmount); @@ -256,10 +254,7 @@ contract BtcMarketPlace { relay.validateProof(transaction, proof); BtcBuyOrder storage order = btcBuyOrders[accept.orderId]; - // Check output script pubkey (recipient address) and amount - uint256 txOutputValue = - BitcoinTx.getTxOutputValue(keccak256(order.bitcoinAddress.scriptPubKey), transaction.outputVector); - assert(txOutputValue >= order.amountBtc); + _checkBitcoinTxOutput(order.amountBtc, order.bitcoinAddress, transaction); IERC20(accept.ercToken).safeTransfer(accept.accepter, accept.ercAmount); @@ -386,4 +381,25 @@ contract BtcMarketPlace { } return (ret, identifiers); } + + /** + * Checks output script pubkey (recipient address) and amount. + * Reverts if transaction amount is lower or bitcoin address is not found. + * + * @param expectedBtcAmount BTC amount requested in order. + * @param bitcoinAddress Recipient's bitcoin address. + * @param transaction Transaction fulfilling the order. + */ + function _checkBitcoinTxOutput( + uint256 expectedBtcAmount, + BitcoinAddress storage bitcoinAddress, + BitcoinTx.Info calldata transaction + ) private { + // Prefixes scriptpubkey with its size to match script output data. + bytes32 b = keccak256(abi.encodePacked(uint8(bitcoinAddress.scriptPubKey.length), bitcoinAddress.scriptPubKey)); + + uint256 txOutputValue = BitcoinTx.getTxOutputValue(b, transaction.outputVector); + + require(txOutputValue >= expectedBtcAmount, "Bitcoin transaction amount is lower than in accepted order."); + } } diff --git a/test/swap/Btc_Marketplace.t.sol b/test/swap/Btc_Marketplace.t.sol index 629f7dd8..13f684d5 100644 --- a/test/swap/Btc_Marketplace.t.sol +++ b/test/swap/Btc_Marketplace.t.sol @@ -61,6 +61,10 @@ contract MarketPlaceTest is BtcMarketPlace, Test { }); } + function dummyBitcoinAddress() public returns (BitcoinAddress memory) { + return BitcoinAddress({scriptPubKey: hex"76a914fd7e6999cd7e7114383e014b7e612a88ab6be68f88ac"}); + } + function testSellBtc() public { token1.sudoMint(bob, 100); @@ -69,7 +73,7 @@ contract MarketPlaceTest is BtcMarketPlace, Test { vm.startPrank(bob); token1.approve(address(this), 40); - this.acceptBtcSellOrder(0, BitcoinAddress({scriptPubKey: "a91476fe4e664d67c35931096d90aabbdbd8e0e11bad87"}), 40); + this.acceptBtcSellOrder(0, dummyBitcoinAddress(), 40); vm.startPrank(alice); this.proofBtcSellOrder(1, dummyTransaction(), dummyProof()); @@ -83,7 +87,7 @@ contract MarketPlaceTest is BtcMarketPlace, Test { vm.startPrank(bob); token1.approve(address(this), 40); - this.acceptBtcSellOrder(0, BitcoinAddress({scriptPubKey: "a91476fe4e664d67c35931096d90aabbdbd8e0e11bad87"}), 40); + this.acceptBtcSellOrder(0, dummyBitcoinAddress(), 40); vm.startPrank(alice); this.withdrawBtcSellOrder(0); @@ -99,7 +103,7 @@ contract MarketPlaceTest is BtcMarketPlace, Test { vm.startPrank(bob); token1.approve(address(this), 40); - this.acceptBtcSellOrder(0, BitcoinAddress({scriptPubKey: "a91476fe4e664d67c35931096d90aabbdbd8e0e11bad87"}), 40); + this.acceptBtcSellOrder(0, dummyBitcoinAddress(), 40); vm.warp(block.timestamp + REQUEST_EXPIRATION_SECONDS + 1); assertEq(token1.balanceOf(address(this)), 4); @@ -114,9 +118,7 @@ contract MarketPlaceTest is BtcMarketPlace, Test { vm.startPrank(alice); token1.approve(address(this), 100); - this.placeBtcBuyOrder( - 1000, BitcoinAddress({scriptPubKey: "a91476fe4e664d67c35931096d90aabbdbd8e0e11bad87"}), address(token1), 100 - ); + this.placeBtcBuyOrder(1000, dummyBitcoinAddress(), address(token1), 100); vm.startPrank(bob); this.acceptBtcBuyOrder(0, 40); @@ -132,9 +134,7 @@ contract MarketPlaceTest is BtcMarketPlace, Test { vm.startPrank(alice); token1.approve(address(this), 100); - this.placeBtcBuyOrder( - 1000, BitcoinAddress({scriptPubKey: "a91476fe4e664d67c35931096d90aabbdbd8e0e11bad87"}), address(token1), 100 - ); + this.placeBtcBuyOrder(1000, dummyBitcoinAddress(), address(token1), 100); vm.startPrank(bob); this.acceptBtcBuyOrder(0, 40); @@ -152,9 +152,7 @@ contract MarketPlaceTest is BtcMarketPlace, Test { vm.startPrank(alice); token1.approve(address(this), 100); - this.placeBtcBuyOrder( - 1000, BitcoinAddress({scriptPubKey: "a91476fe4e664d67c35931096d90aabbdbd8e0e11bad87"}), address(token1), 100 - ); + this.placeBtcBuyOrder(1000, dummyBitcoinAddress(), address(token1), 100); vm.startPrank(bob); this.acceptBtcBuyOrder(0, 40);