Merge pull request #112 from line/apply_v0.33.6

Apply tendermint v0.33.6

CHANGELOG_OF_TENDERMINT.md

@@ -1,5 +1,65 @@
 # Changelog
 
+## v0.33.6
+
+*July 2, 2020*
+
+This security release fixes:
+
+### Denial of service
+
+Tendermint 0.33.0 and above allow block proposers to include signatures for the
+wrong block. This may happen naturally if you start a network, have it run for
+some time and restart it **without changing the chainID**. (It is a 
+[misconfiguration](https://docs.tendermint.com/master/tendermint-core/using-tendermint.html) 
+to reuse chainIDs.) Correct block proposers will accidentally include signatures 
+for the wrong block if they see these signatures, and then commits won't validate, 
+making all proposed blocks invalid. A malicious validator (even with a minimal 
+amount of stake) can use this vulnerability to completely halt the network. 
+
+Tendermint 0.33.6 checks all the signatures are for the block with +2/3
+majority before creating a commit.
+
+### False Witness
+
+Tendermint 0.33.1 and above are no longer fully verifying commit signatures
+during block execution - they stop after +2/3. This means proposers can propose
+blocks that contain valid +2/3 signatures and then the rest of the signatures
+can be whatever they want. They can claim that all the other validators signed
+just by including a CommitSig with arbitrary signature data. While this doesn't
+seem to impact safety of Tendermint per se, it means that Commits may contain a
+lot of invalid data.
+
+_This was already true of blocks, since they could include invalid txs filled
+with garbage, but in that case the application knew that they are invalid and
+could punish the proposer. But since applications didn't--and don't--
+verify commit signatures directly (they trust Tendermint to do that), 
+they won't be able to detect it._
+
+This can impact incentivization logic in the application that depends on the
+LastCommitInfo sent in BeginBlock, which includes which validators signed. For
+instance, Gaia incentivizes proposers with a bonus for including more than +2/3
+of the signatures. But a proposer can now claim that bonus just by including
+arbitrary data for the final -1/3 of validators without actually waiting for
+their signatures. There may be other tricks that can be played because of this.
+
+Tendermint 0.33.6 verifies all the signatures during block execution.
+
+_Please note that the light client does not check nil votes and exits as soon 
+as 2/3+ of the signatures are checked._
+
+**All clients are recommended to upgrade.**
+
+Special thanks to @njmurarka at Bluzelle Networks for reporting this.
+
+Friendly reminder, we have a [bug bounty
+program](https://hackerone.com/tendermint).
+
+### SECURITY:
+
+- [consensus] Do not allow signatures for a wrong block in commits (@ebuchman)
+- [consensus] Verify all the signatures during block execution (@melekes)
+
 ## v.0.33.5
 
 Special thanks to our external contributor on this release: @tau3

consensus/invalid_test.go

@@ -0,0 +1,94 @@
+package consensus
+
+import (
+	"testing"
+
+	"github.com/tendermint/tendermint/libs/bytes"
+	"github.com/tendermint/tendermint/libs/log"
+	tmrand "github.com/tendermint/tendermint/libs/rand"
+	"github.com/tendermint/tendermint/p2p"
+	"github.com/tendermint/tendermint/types"
+)
+
+//----------------------------------------------
+// byzantine failures
+
+// one byz val sends a precommit for a random block at each height
+// Ensure a testnet makes blocks
+func TestReactorInvalidPrecommit(t *testing.T) {
+	N := 4
+	css, cleanup := randConsensusNet(N, "consensus_reactor_test", newMockTickerFunc(true), newCounter)
+	defer cleanup()
+
+	for i := 0; i < 4; i++ {
+		ticker := NewTimeoutTicker()
+		ticker.SetLogger(css[i].Logger)
+		css[i].SetTimeoutTicker(ticker)
+
+	}
+
+	reactors, blocksSubs, eventBuses := startConsensusNet(t, css, N)
+
+	// this val sends a random precommit at each height
+	byzValIdx := 0
+	byzVal := css[byzValIdx]
+	byzR := reactors[byzValIdx]
+
+	// update the doPrevote function to just send a valid precommit for a random block
+	// and otherwise disable the priv validator
+	byzVal.mtx.Lock()
+	pv := byzVal.privValidator
+	byzVal.doPrevote = func(height int64, round int) {
+		invalidDoPrevoteFunc(t, height, round, byzVal, byzR.Switch, pv)
+	}
+	byzVal.mtx.Unlock()
+	defer stopConsensusNet(log.TestingLogger(), reactors, eventBuses)
+
+	// wait for a bunch of blocks
+	// TODO: make this tighter by ensuring the halt happens by block 2
+	for i := 0; i < 10; i++ {
+		timeoutWaitGroup(t, N, func(j int) {
+			<-blocksSubs[j].Out()
+		}, css)
+	}
+}
+
+func invalidDoPrevoteFunc(t *testing.T, height int64, round int, cs *State, sw *p2p.Switch, pv types.PrivValidator) {
+	// routine to:
+	// - precommit for a random block
+	// - send precommit to all peers
+	// - disable privValidator (so we don't do normal precommits)
+	go func() {
+		cs.mtx.Lock()
+		cs.privValidator = pv
+		pubKey, err := cs.privValidator.GetPubKey()
+		if err != nil {
+			panic(err)
+		}
+		addr := pubKey.Address()
+		valIndex, _ := cs.Validators.GetByAddress(addr)
+
+		// precommit a random block
+		blockHash := bytes.HexBytes(tmrand.Bytes(32))
+		precommit := &types.Vote{
+			ValidatorAddress: addr,
+			ValidatorIndex:   valIndex,
+			Height:           cs.Height,
+			Round:            cs.Round,
+			Timestamp:        cs.voteTime(),
+			Type:             types.PrecommitType,
+			BlockID: types.BlockID{
+				Hash:        blockHash,
+				PartsHeader: types.PartSetHeader{Total: 1, Hash: tmrand.Bytes(32)}},
+		}
+		cs.privValidator.SignVote(cs.state.ChainID, precommit)
+		cs.privValidator = nil // disable priv val so we don't do normal votes
+		cs.mtx.Unlock()
+
+		peers := sw.Peers().List()
+		for _, peer := range peers {
+			cs.Logger.Info("Sending bad vote", "block", blockHash, "peer", peer)
+			peer.Send(VoteChannel, cdc.MustMarshalBinaryBare(&VoteMessage{precommit}))
+		}
+	}()
+}

lite2/client.go

@@ -404,7 +404,7 @@ func (c *Client) initializeWithTrustOptions(options TrustOptions) error {
 	voters := types.SelectVoter(vals, proofHash, c.voterParams)
 
 	// Ensure that +2/3 of voters signed correctly.
-	err = voters.VerifyCommit(c.chainID, h.Commit.BlockID, h.Height, h.Commit)
+	err = voters.VerifyCommitLight(c.chainID, h.Commit.BlockID, h.Height, h.Commit)
 	if err != nil {
 		return fmt.Errorf("invalid commit: %w", err)
 	}
@@ -977,7 +977,7 @@ func (c *Client) compareNewHeaderWithWitnesses(h *types.SignedHeader) error {
 					c.logger.Error("Witness sent us incorrect header; invalid proof", "err", err)
 				}
 				voters := types.SelectVoter(c.latestTrustedValidators, proofHash, c.voterParams)
-				if err = voters.VerifyCommitTrusting(c.chainID, altH.Commit.BlockID,
+				if err = voters.VerifyCommitLightTrusting(c.chainID, altH.Commit.BlockID,
 					altH.Height, altH.Commit, c.trustLevel); err != nil {
 					c.logger.Error("Witness sent us incorrect header", "err", err, "witness", witness)
 					witnessesToRemove = append(witnessesToRemove, i)

lite2/verifier.go

@@ -69,7 +69,7 @@ func VerifyNonAdjacent(
 	}
 
 	// Ensure that +`trustLevel` (default 1/3) or more of last trusted validators signed correctly.
-	err = trustedVoters.VerifyCommitTrusting(
+	err = trustedVoters.VerifyCommitLightTrusting(
 		chainID,
 		untrustedHeader.Commit.BlockID,
 		untrustedHeader.Height,
@@ -89,7 +89,7 @@ func VerifyNonAdjacent(
 	// NOTE: this should always be the last check because untrustedVals can be
 	// intentionally made very large to DOS the light client. not the case for
 	// VerifyAdjacent, where validator set is known in advance.
-	if err := untrustedVoters.VerifyCommit(
+	if err := untrustedVoters.VerifyCommitLight(
 		chainID,
 		untrustedHeader.Commit.BlockID,
 		untrustedHeader.Height,
@@ -159,7 +159,7 @@ func VerifyAdjacent(
 	}
 
 	// Ensure that +2/3 of new validators signed correctly.
-	if err := untrustedVoters.VerifyCommit(
+	if err := untrustedVoters.VerifyCommitLight(
 		chainID,
 		untrustedHeader.Commit.BlockID,
 		untrustedHeader.Height,