Skip to content

Commit

Permalink
Merge pull request #2587 from ManfredKarrer/add-check-for-bsq-balance
Browse files Browse the repository at this point in the history
Add irregular txType, add check for total balance, prevent proposal withhold attack
  • Loading branch information
ManfredKarrer authored Mar 31, 2019
2 parents c876269 + f95f770 commit 3854907
Show file tree
Hide file tree
Showing 91 changed files with 893 additions and 362 deletions.
83 changes: 83 additions & 0 deletions common/src/main/java/bisq/common/util/ExtraDataMapValidator.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
* This file is part of Bisq.
*
* Bisq is free software: you can redistribute it and/or modify it
* under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or (at
* your option) any later version.
*
* Bisq is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public
* License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with Bisq. If not, see <http://www.gnu.org/licenses/>.
*/

package bisq.common.util;

import java.util.HashMap;
import java.util.Map;

import lombok.extern.slf4j.Slf4j;

import javax.annotation.Nullable;

import static com.google.common.base.Preconditions.checkArgument;

/**
* Validator for extraDataMap fields used in network payloads.
* Ensures that we don't get the network attacked by huge data inserted there.
*/
@Slf4j
public class ExtraDataMapValidator {
// ExtraDataMap is only used for exceptional cases to not break backward compatibility.
// We don't expect many entries there.
public final static int MAX_SIZE = 10;
public final static int MAX_KEY_LENGTH = 100;
public final static int MAX_VALUE_LENGTH = 100000; // 100 kb

public static Map<String, String> getValidatedExtraDataMap(@Nullable Map<String, String> extraDataMap) {
return getValidatedExtraDataMap(extraDataMap, MAX_SIZE, MAX_KEY_LENGTH, MAX_VALUE_LENGTH);
}

public static Map<String, String> getValidatedExtraDataMap(@Nullable Map<String, String> extraDataMap, int maxSize,
int maxKeyLength, int maxValueLength) {
if (extraDataMap == null)
return null;

try {
checkArgument(extraDataMap.entrySet().size() <= maxSize,
"Size of map must not exceed " + maxSize);
extraDataMap.forEach((key, value) -> {
checkArgument(key.length() <= maxKeyLength,
"Length of key must not exceed " + maxKeyLength);
checkArgument(value.length() <= maxValueLength,
"Length of value must not exceed " + maxValueLength);
});
return extraDataMap;
} catch (Throwable t) {
return new HashMap<>();
}
}

public static void validate(@Nullable Map<String, String> extraDataMap) {
validate(extraDataMap, MAX_SIZE, MAX_KEY_LENGTH, MAX_VALUE_LENGTH);
}

public static void validate(@Nullable Map<String, String> extraDataMap, int maxSize, int maxKeyLength,
int maxValueLength) {
if (extraDataMap == null)
return;

checkArgument(extraDataMap.entrySet().size() <= maxSize,
"Size of map must not exceed " + maxSize);
extraDataMap.forEach((key, value) -> {
checkArgument(key.length() <= maxKeyLength,
"Length of key must not exceed " + maxKeyLength);
checkArgument(value.length() <= maxValueLength,
"Length of value must not exceed " + maxValueLength);
});
}
}
3 changes: 2 additions & 1 deletion common/src/main/proto/pb.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1420,7 +1420,7 @@ message Tx {
// Because of the way how PB implements inheritence we need to use the super class as type
repeated BaseTxOutput tx_outputs = 1;
TxType txType = 2;
int64 burnt_fee = 3;
int64 burnt_bsq = 3;
}

enum TxType {
Expand All @@ -1440,6 +1440,7 @@ enum TxType {
UNLOCK = 13;
ASSET_LISTING_FEE = 14;
PROOF_OF_BURN = 15;
IRREGULAR = 16;
}

message TxInput {
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/java/bisq/core/alert/Alert.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import bisq.common.app.Version;
import bisq.common.crypto.Sig;
import bisq.common.util.ExtraDataMapValidator;

import io.bisq.generated.protobuffer.PB;

Expand Down Expand Up @@ -91,7 +92,7 @@ public Alert(String message,
this.version = version;
this.ownerPubKeyBytes = ownerPubKeyBytes;
this.signatureAsBase64 = signatureAsBase64;
this.extraDataMap = extraDataMap;
this.extraDataMap = ExtraDataMapValidator.getValidatedExtraDataMap(extraDataMap);

ownerPubKey = Sig.getPublicKeyFromBytes(ownerPubKeyBytes);
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/bisq/core/app/BisqExecutable.java
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ protected void customizeOptionParsing(OptionParser parser) {
format("Base currency network (default: %s)", BisqEnvironment.getDefaultBaseCurrencyNetwork().name()))
.withRequiredArg()
.ofType(String.class)
.describedAs(format("%s|%s|%s|%s", BTC_MAINNET, BTC_TESTNET, BTC_REGTEST, BTC_DAO_TESTNET, BTC_DAO_BETANET));
.describedAs(format("%s|%s|%s|%s", BTC_MAINNET, BTC_TESTNET, BTC_REGTEST, BTC_DAO_TESTNET, BTC_DAO_BETANET, BTC_DAO_REGTEST));

parser.accepts(BtcOptionKeys.REG_TEST_HOST,
format("Bitcoin regtest host when using BTC_REGTEST network (default: %s)", RegTestHost.DEFAULT_HOST))
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/bisq/core/app/BisqSetup.java
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ private void maybeShowTac() {

private void checkIfLocalHostNodeIsRunning() {
// For DAO testnet we ignore local btc node
if (BisqEnvironment.getBaseCurrencyNetwork().isDaoTestNet()) {
if (BisqEnvironment.getBaseCurrencyNetwork().isDaoRegTest() || BisqEnvironment.getBaseCurrencyNetwork().isDaoTestNet()) {
step3();
} else {
Thread checkIfLocalHostNodeIsRunningThread = new Thread(() -> {
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/java/bisq/core/arbitration/Arbitrator.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import bisq.common.crypto.PubKeyRing;
import bisq.common.proto.ProtoUtil;
import bisq.common.util.ExtraDataMapValidator;
import bisq.common.util.Utilities;

import io.bisq.generated.protobuffer.PB;
Expand Down Expand Up @@ -91,7 +92,7 @@ public Arbitrator(NodeAddress nodeAddress,
this.registrationSignature = registrationSignature;
this.emailAddress = emailAddress;
this.info = info;
this.extraDataMap = extraDataMap;
this.extraDataMap = ExtraDataMapValidator.getValidatedExtraDataMap(extraDataMap);
}

///////////////////////////////////////////////////////////////////////////////////////////
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/java/bisq/core/arbitration/Mediator.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

import bisq.common.crypto.PubKeyRing;
import bisq.common.proto.ProtoUtil;
import bisq.common.util.ExtraDataMapValidator;

import io.bisq.generated.protobuffer.PB;

Expand Down Expand Up @@ -84,7 +85,7 @@ public Mediator(NodeAddress nodeAddress,
this.registrationSignature = registrationSignature;
this.emailAddress = emailAddress;
this.info = info;
this.extraDataMap = extraDataMap;
this.extraDataMap = ExtraDataMapValidator.getValidatedExtraDataMap(extraDataMap);
}


Expand Down
9 changes: 7 additions & 2 deletions core/src/main/java/bisq/core/btc/BaseCurrencyNetwork.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ public enum BaseCurrencyNetwork {
BTC_MAINNET(MainNetParams.get(), "BTC", "MAINNET", "Bitcoin"),
BTC_TESTNET(TestNet3Params.get(), "BTC", "TESTNET", "Bitcoin"),
BTC_REGTEST(RegTestParams.get(), "BTC", "REGTEST", "Bitcoin"),
BTC_DAO_TESTNET(RegTestParams.get(), "BTC", "REGTEST", "Bitcoin"), // server side regtest
BTC_DAO_BETANET(MainNetParams.get(), "BTC", "MAINNET", "Bitcoin"); // mainnet test genesis
BTC_DAO_TESTNET(RegTestParams.get(), "BTC", "REGTEST", "Bitcoin"), // server side regtest until v0.9.5
BTC_DAO_BETANET(MainNetParams.get(), "BTC", "MAINNET", "Bitcoin"), // mainnet test genesis
BTC_DAO_REGTEST(RegTestParams.get(), "BTC", "REGTEST", "Bitcoin"); // server side regtest after v0.9.5, had breaking code changes so we started over again

@Getter
private final NetworkParameters parameters;
Expand Down Expand Up @@ -59,6 +60,10 @@ public boolean isDaoTestNet() {
return "BTC_DAO_TESTNET".equals(name());
}

public boolean isDaoRegTest() {
return "BTC_DAO_REGTEST".equals(name());
}

public boolean isDaoBetaNet() {
return "BTC_DAO_BETANET".equals(name());
}
Expand Down
8 changes: 5 additions & 3 deletions core/src/main/java/bisq/core/btc/BitcoinModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,15 @@ public BitcoinModule(Environment environment) {

@Override
protected void configure() {
// We we have selected BTC_DAO_TESTNET we use our master regtest node, otherwise the specified host or default
// (localhost)
// If we we have selected BTC_DAO_REGTEST or BTC_DAO_TESTNET we use our master regtest node,
// otherwise the specified host or default (localhost)
String regTestHost = environment.getProperty(BtcOptionKeys.REG_TEST_HOST, String.class, "");
if (regTestHost.isEmpty()) {
regTestHost = BisqEnvironment.getBaseCurrencyNetwork().isDaoTestNet() ?
"104.248.31.39" :
RegTestHost.DEFAULT_HOST;
BisqEnvironment.getBaseCurrencyNetwork().isDaoRegTest() ?
"134.209.242.206" :
RegTestHost.DEFAULT_HOST;
}

RegTestHost.HOST = regTestHost;
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/bisq/core/btc/setup/WalletConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ private PeerGroup createPeerGroup() {

// For dao testnet (server side regtest) we prevent to connect to a localhost node to avoid confusion
// if local btc node is not synced with our dao testnet master node.
if (BisqEnvironment.getBaseCurrencyNetwork().isDaoTestNet())
if (BisqEnvironment.getBaseCurrencyNetwork().isDaoRegTest() || BisqEnvironment.getBaseCurrencyNetwork().isDaoTestNet())
peerGroup.setUseLocalhostPeerWhenPossible(false);

return peerGroup;
Expand Down
25 changes: 24 additions & 1 deletion core/src/main/java/bisq/core/dao/DaoFacade.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
import bisq.core.dao.state.DaoStateService;
import bisq.core.dao.state.DaoStateStorageService;
import bisq.core.dao.state.model.blockchain.BaseTx;
import bisq.core.dao.state.model.blockchain.BaseTxOutput;
import bisq.core.dao.state.model.blockchain.Block;
import bisq.core.dao.state.model.blockchain.Tx;
import bisq.core.dao.state.model.blockchain.TxOutput;
Expand Down Expand Up @@ -543,6 +544,28 @@ public long getTotalAmountOfConfiscatedTxOutputs() {
return daoStateService.getTotalAmountOfConfiscatedTxOutputs();
}

public long getTotalAmountOfInvalidatedBsq() {
return daoStateService.getTotalAmountOfInvalidatedBsq();
}

// Contains burned fee and invalidated bsq due invalid txs
public long getTotalAmountOfBurntBsq() {
return daoStateService.getTotalAmountOfBurntBsq();
}

public List<Tx> getInvalidTxs() {
return daoStateService.getInvalidTxs();
}

public List<Tx> getIrregularTxs() {
return daoStateService.getIrregularTxs();
}

public long getTotalAmountOfUnspentTxOutputs() {
// Does not consider confiscated outputs (they stay as utxo)
return daoStateService.getUnspentTxOutputMap().values().stream().mapToLong(BaseTxOutput::getValue).sum();
}

public Optional<Integer> getLockTime(String txId) {
return daoStateService.getLockTime(txId);
}
Expand Down Expand Up @@ -589,7 +612,7 @@ public int getNumIssuanceTransactions(IssuanceType issuanceType) {
return daoStateService.getIssuanceSet(issuanceType).size();
}

public Set<Tx> getFeeTxs() {
public Set<Tx> getBurntFeeTxs() {
return daoStateService.getBurntFeeTxs();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
public class BallotListPresentation implements BallotListService.BallotListChangeListener, DaoStateListener {
private final BallotListService ballotListService;
private final PeriodService periodService;
private final DaoStateService daoStateService;
private final ProposalValidatorProvider proposalValidatorProvider;

@Getter
Expand All @@ -65,31 +64,20 @@ public BallotListPresentation(BallotListService ballotListService,
ProposalValidatorProvider proposalValidatorProvider) {
this.ballotListService = ballotListService;
this.periodService = periodService;
this.daoStateService = daoStateService;
this.proposalValidatorProvider = proposalValidatorProvider;

daoStateService.addDaoStateListener(this);
ballotListService.addListener(this);
}


///////////////////////////////////////////////////////////////////////////////////////////
// DaoStateListener
///////////////////////////////////////////////////////////////////////////////////////////

@Override
public void onNewBlockHeight(int blockHeight) {
if (daoStateService.isParseBlockChainComplete()) {
ballotsOfCycle.setPredicate(ballot -> periodService.isTxInCorrectCycle(ballot.getTxId(), blockHeight));
}
}

@Override
public void onParseBlockChainComplete() {
ballotsOfCycle.setPredicate(ballot -> periodService.isTxInCorrectCycle(ballot.getTxId(), daoStateService.getChainHeight()));
}

@Override
public void onParseBlockCompleteAfterBatchProcessing(Block block) {
ballotsOfCycle.setPredicate(ballot -> periodService.isTxInCorrectCycle(ballot.getTxId(), block.getHeight()));
onListChanged(ballotListService.getValidatedBallotList());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import bisq.common.proto.network.NetworkPayload;
import bisq.common.proto.persistable.PersistablePayload;
import bisq.common.util.ExtraDataMapValidator;
import bisq.common.util.Utilities;

import io.bisq.generated.protobuffer.PB;
Expand Down Expand Up @@ -65,7 +66,7 @@ public BlindVote(byte[] encryptedVotes,
this.txId = txId;
this.stake = stake;
this.encryptedMeritList = encryptedMeritList;
this.extraDataMap = extraDataMap;
this.extraDataMap = ExtraDataMapValidator.getValidatedExtraDataMap(extraDataMap);
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,20 +136,20 @@ private void fillListFromAppendOnlyDataStore() {
p2PService.getP2PDataStorage().getAppendOnlyDataStoreMap().values().forEach(e -> onAppendOnlyDataAdded(e, false));
}

private void onAppendOnlyDataAdded(PersistableNetworkPayload persistableNetworkPayload, boolean doLog) {
private void onAppendOnlyDataAdded(PersistableNetworkPayload persistableNetworkPayload, boolean fromBroadcastMessage) {
if (persistableNetworkPayload instanceof BlindVotePayload) {
BlindVotePayload blindVotePayload = (BlindVotePayload) persistableNetworkPayload;
if (!blindVotePayloads.contains(blindVotePayload)) {
BlindVote blindVote = blindVotePayload.getBlindVote();
String txId = blindVote.getTxId();
// We don't check the phase and the cycle as we want to add all object independently when we receive it
// (or when we start the app to fill our list from the data we gor from the seed node).
// We don't validate as we might receive blindVotes from other cycles or phases at startup.
// Beside that we might receive payloads we requested at the vote result phase in case we missed some
// payloads. We prefer here resilience over protection against late publishing attacks.
if (blindVoteValidator.areDataFieldsValid(blindVote)) {
// We don't validate as we might receive blindVotes from other cycles or phases at startup.
blindVotePayloads.add(blindVotePayload);
if (doLog) {
if (fromBroadcastMessage) {
log.info("We received a blindVotePayload. blindVoteTxId={}", txId);
}
blindVotePayloads.add(blindVotePayload);
} else {
log.warn("We received an invalid blindVotePayload. blindVoteTxId={}", txId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,15 @@

package bisq.core.dao.governance.blindvote;

import bisq.core.btc.wallet.Restrictions;
import bisq.core.dao.governance.period.PeriodService;
import bisq.core.dao.governance.proposal.ProposalValidationException;
import bisq.core.dao.state.DaoStateService;
import bisq.core.dao.state.model.blockchain.Tx;
import bisq.core.dao.state.model.governance.DaoPhase;

import bisq.common.util.ExtraDataMapValidator;

import javax.inject.Inject;

import java.util.Optional;
Expand Down Expand Up @@ -58,12 +61,20 @@ private void validateDataFields(BlindVote blindVote) throws ProposalValidationEx
checkNotNull(blindVote.getEncryptedVotes(), "encryptedProposalList must not be null");
checkArgument(blindVote.getEncryptedVotes().length > 0,
"encryptedProposalList must not be empty");
checkNotNull(blindVote.getTxId(), "txId must not be null");
checkArgument(!blindVote.getTxId().isEmpty(), "txId must not be empty");
checkArgument(blindVote.getStake() > 0, "stake must be positive");
checkArgument(blindVote.getEncryptedVotes().length <= 100000,
"encryptedProposalList must not exceed 100kb");

checkNotNull(blindVote.getTxId(), "Tx ID must not be null");
checkArgument(blindVote.getTxId().length() == 64, "Tx ID must be 64 chars");
checkArgument(blindVote.getStake() >= Restrictions.getMinNonDustOutput().value, "Stake must be at least MinNonDustOutput");

checkNotNull(blindVote.getEncryptedMeritList(), "getEncryptedMeritList must not be null");
checkArgument(blindVote.getEncryptedMeritList().length > 0,
"getEncryptedMeritList must not be empty");
checkArgument(blindVote.getEncryptedMeritList().length <= 100000,
"getEncryptedMeritList must not exceed 100kb");

ExtraDataMapValidator.validate(blindVote.getExtraDataMap());
} catch (Throwable e) {
log.warn(e.toString());
throw new ProposalValidationException(e);
Expand Down
Loading

0 comments on commit 3854907

Please sign in to comment.