mirrors/bisq
1
0
Fork 0
mirror of https://github.com/bisq-network/bisq.git synced 2024-11-19 18:03:12 +01:00
Code Issues Packages Projects Releases Wiki Activity

Fix BSQ swap buyer tx fee theft vulnerability

Browse Source 
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 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 commit is contained in:
Steven Barclay 2021-12-02 17:19:10 +00:00
parent 8d267520c8
commit 915a79e627
No known key found for this signature in database
GPG Key ID: 9FED6BF1176D500B
2 changed files with 31 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
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,17 @@ 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);
}
@Override

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");