mirrors/bisq
1
0
Fork 0
mirror of https://github.com/bisq-network/bisq.git synced 2025-02-24 23:18:17 +01:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #5889 from stejbac/fix-bsq-swap-tx-fee-theft-vulnerability

Browse source 
Fix BSQ swap buyer tx fee theft vulnerability
This commit is contained in:
Christoph Atteneder 2021-12-04 19:50:50 +01:00 • committed by GitHub
commit 628ac27e25
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 43 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

26
core/src/main/java/bisq/core/btc/model/RawTransactionInput.java
View file

@ -17,7 +17,7 @@
package bisq.core.btc.model;
import bisq.core.btc.wallet.BtcWalletService;
import bisq.core.btc.wallet.WalletService;
import bisq.common.proto.network.NetworkPayload;
import bisq.common.proto.persistable.PersistablePayload;
@ -36,6 +36,10 @@ import lombok.Getter;
import javax.annotation.concurrent.Immutable;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkElementIndex;
import static com.google.common.base.Preconditions.checkNotNull;
@EqualsAndHashCode
@Immutable
@Getter
@ -56,7 +60,7 @@ public final class RawTransactionInput implements NetworkPayload, PersistablePay
input.getConnectedOutput() != null &&
input.getConnectedOutput().getScriptPubKey() != null &&
input.getConnectedOutput().getScriptPubKey().getScriptType() != null ?
input.getConnectedOutput().getScriptPubKey().getScriptType().id : -1);
input.getConnectedOutput().getScriptPubKey().getScriptType().id : 0);
}
// Does not set the scriptTypeId. Use RawTransactionInput(TransactionInput input) for any new code.
@ -129,8 +133,22 @@ public final class RawTransactionInput implements NetworkPayload, PersistablePay
return scriptTypeId == Script.ScriptType.P2WSH.id;
}
public String getParentTxId(BtcWalletService btcWalletService) {
return btcWalletService.getTxFromSerializedTx(parentTransaction).getTxId().toString();
public String getParentTxId(WalletService walletService) {
return walletService.getTxFromSerializedTx(parentTransaction).getTxId().toString();
}
public void validate(WalletService walletService) {
Transaction tx = walletService.getTxFromSerializedTx(checkNotNull(parentTransaction));
checkArgument(index == (int) index, "Input index out of range.");
checkElementIndex((int) index, tx.getOutputs().size(), "Input index");
long outputValue = tx.getOutput(index).getValue().value;
checkArgument(value == outputValue,
"Input value (%s) mismatches connected tx output value (%s).", value, outputValue);
var scriptPubKey = tx.getOutput(index).getScriptPubKey();
var scriptType = scriptPubKey != null ? scriptPubKey.getScriptType() : null;
checkArgument(scriptTypeId <= 0 || scriptType != null && scriptType.id == scriptTypeId,
"Input scriptTypeId (%s) mismatches connected tx output scriptTypeId (%s.id = %s).",
scriptTypeId, scriptType, scriptType != null ? scriptType.id : 0);
}
@Override

8
core/src/main/java/bisq/core/trade/bsq_swap/BsqSwapTakeOfferRequestVerification.java
View file

@ -77,7 +77,7 @@ public class BsqSwapTakeOfferRequestVerification {
checkArgument(isDateInTolerance(request), "Trade date is out of tolerance");
checkArgument(isTxFeeInTolerance(request, feeService), "Miner fee from taker not in tolerance");
checkArgument(request.getMakerFee() == Objects.requireNonNull(CoinUtil.getMakerFee(false, amountAsCoin)).value);
checkArgument(request.getTakerFee() == CoinUtil.getTakerFee(false, amountAsCoin).value);
checkArgument(request.getTakerFee() == Objects.requireNonNull(CoinUtil.getTakerFee(false, amountAsCoin)).value);
} catch (Exception e) {
log.error("BsqSwapTakeOfferRequestVerification failed. Request={}, peer={}, error={}", request, peer, e.toString());
return false;
@ -93,9 +93,9 @@ public class BsqSwapTakeOfferRequestVerification {
private static boolean isTxFeeInTolerance(BsqSwapRequest request, FeeService feeService) {
double myFee = (double) feeService.getTxFeePerVbyte().getValue();
double peersFee = (double) Coin.valueOf(request.getTxFeePerVbyte()).getValue();
// Allow for 10% diff in mining fee, ie, maker will accept taker fee that's 10%
// off their own fee from service. Both parties will use the same fee while
// creating the bsq swap tx
// Allow for 50% diff in mining fee, ie, maker will accept taker fee that's less
// than 50% off their own fee from service (that is, 100% higher or 50% lower).
// Both parties will use the same fee while creating the bsq swap tx.
double diff = abs(1 - myFee / peersFee);
boolean isInTolerance = diff < 0.5;
if (!isInTolerance) {

19
core/src/main/java/bisq/core/trade/protocol/bsq_swap/tasks/buyer/ProcessBsqSwapFinalizeTxRequest.java
View file

@ -19,6 +19,7 @@ package bisq.core.trade.protocol.bsq_swap.tasks.buyer;
import bisq.core.btc.model.RawTransactionInput;
import bisq.core.btc.wallet.Restrictions;
import bisq.core.btc.wallet.WalletService;
import bisq.core.trade.bsq_swap.BsqSwapCalculation;
import bisq.core.trade.model.bsq_swap.BsqSwapTrade;
import bisq.core.trade.protocol.bsq_swap.messages.BsqSwapFinalizeTxRequest;
@ -66,11 +67,13 @@ public abstract class ProcessBsqSwapFinalizeTxRequest extends BsqSwapTask {
checkNotNull(request);
Validator.checkTradeId(protocolModel.getOfferId(), request);
// We will use only the sellers buyersBsqInputs from the tx so we do not verify anything else
// We will use only the seller's BTC inputs from the tx, so we do not verify anything else.
byte[] tx = request.getTx();
Transaction sellersTransaction = protocolModel.getBtcWalletService().getTxFromSerializedTx(tx);
WalletService btcWalletService = protocolModel.getBtcWalletService();
Transaction sellersTransaction = btcWalletService.getTxFromSerializedTx(tx);
List<RawTransactionInput> sellersRawBtcInputs = request.getBtcInputs();
checkArgument(!sellersRawBtcInputs.isEmpty(), "Sellers BTC buyersBsqInputs must not be empty");
checkArgument(!sellersRawBtcInputs.isEmpty(), "SellersRawBtcInputs must not be empty");
sellersRawBtcInputs.forEach(input -> input.validate(btcWalletService));
List<RawTransactionInput> buyersBsqInputs = protocolModel.getInputs();
int buyersInputSize = Objects.requireNonNull(buyersBsqInputs).size();
@ -78,7 +81,13 @@ public abstract class ProcessBsqSwapFinalizeTxRequest extends BsqSwapTask {
.filter(input -> input.getIndex() >= buyersInputSize)
.collect(Collectors.toList());
checkArgument(sellersBtcInputs.size() == sellersRawBtcInputs.size(),
"Number of buyersBsqInputs in tx must match the number of sellersRawBtcInputs");
"Number of sellersBtcInputs in tx must match the number of sellersRawBtcInputs");
for (int i = 0; i < sellersBtcInputs.size(); i++) {
String parentTxId = sellersBtcInputs.get(i).getOutpoint().getHash().toString();
String rawParentTxId = sellersRawBtcInputs.get(i).getParentTxId(btcWalletService);
checkArgument(parentTxId.equals(rawParentTxId),
"Spending tx mismatch between sellersBtcInputs and sellersRawBtcInputs at index %s", i);
}
boolean hasUnSignedInputs = sellersBtcInputs.stream()
.anyMatch(input -> input.getScriptSig() == null && !input.hasWitness());
@ -113,7 +122,7 @@ public abstract class ProcessBsqSwapFinalizeTxRequest extends BsqSwapTask {
checkArgument(change <= expectedChange,
"Change must be smaller or equal to expectedChange");
NetworkParameters params = protocolModel.getBtcWalletService().getParams();
NetworkParameters params = btcWalletService.getParams();
String sellersBsqPayoutAddress = request.getBsqPayoutAddress();
checkNotNull(sellersBsqPayoutAddress, "sellersBsqPayoutAddress must not be null");
checkArgument(!sellersBsqPayoutAddress.isEmpty(), "sellersBsqPayoutAddress must not be empty");

4
core/src/main/java/bisq/core/trade/protocol/bsq_swap/tasks/seller/ProcessTxInputsMessage.java
View file

@ -62,8 +62,11 @@ public abstract class ProcessTxInputsMessage extends BsqSwapTask {
TxInputsMessage message = checkNotNull((TxInputsMessage) protocolModel.getTradeMessage());
checkNotNull(message);
BtcWalletService btcWalletService = protocolModel.getBtcWalletService();
List<RawTransactionInput> inputs = message.getBsqInputs();
checkArgument(!inputs.isEmpty(), "Buyers BSQ inputs must not be empty");
inputs.forEach(input -> input.validate(btcWalletService));
long sumInputs = inputs.stream().mapToLong(rawTransactionInput -> rawTransactionInput.value).sum();
long buyersBsqInputAmount = BsqSwapCalculation.getBuyersBsqInputValue(trade, getBuyersTradeFee()).getValue();
@ -71,7 +74,6 @@ public abstract class ProcessTxInputsMessage extends BsqSwapTask {
"Buyers BSQ input amount do not match our calculated required BSQ input amount");
DaoFacade daoFacade = protocolModel.getDaoFacade();
BtcWalletService btcWalletService = protocolModel.getBtcWalletService();
long sumValidBsqInputValue = inputs.stream()
.mapToLong(input -> daoFacade.getUnspentTxOutputValue(