fix test

add ValidateBasic to crypto/merkle/SimpleProof

CHANGELOG.md

@@ -1,5 +1,73 @@
 # Changelog
 
+## v0.32.7
+
+*October 18, 2019*
+
+This security release fixes a vulnerability found in the `consensus` package,
+where an attacker could construct a `BlockPartMessage` message in such a way
+that it will lead to consensus failure. A few similar issues have been
+identified and fixed here.
+
+**All clients are recommended to upgrade**
+
+Special thanks to [elvishacker](https://hackerone.com/elvishacker) for finding
+and reporting this.
+
+Friendly reminder, we have a [bug bounty
+program](https://hackerone.com/tendermint).
+
+### BREAKING CHANGES:
+
+- Go API
+  - [consensus] Modify `WAL#Write` and `WAL#WriteSync` to return an error if
+    they fail to write a message
+
+### SECURITY:
+
+- [consensus] Validate incoming messages more throughly
+
+## v0.32.6
+
+*October 8, 2019*
+
+The previous patch was insufficient because the attacker could still find a way
+to submit a `nil` pubkey by constructing a `PubKeyMultisigThreshold` pubkey
+with `nil` subpubkeys for example.
+
+This release provides multiple fixes, which include recovering from panics when
+accepting new peers and only allowing `ed25519` pubkeys.
+
+**All clients are recommended to upgrade**
+
+Special thanks to [fudongbai](https://hackerone.com/fudongbai) for pointing
+this out.
+
+Friendly reminder, we have a [bug bounty
+program](https://hackerone.com/tendermint).
+
+### SECURITY:
+
+- [p2p] [\#4030](https://github.com/tendermint/tendermint/issues/4030) Only allow ed25519 pubkeys when connecting
+
+## v0.32.5
+
+*September 30, 2019*
+
+This release fixes a major security vulnerability found in the `p2p` package.
+All clients are recommended to upgrade. See [TODO](hxxp://githublink) for
+details.
+
+Special thanks to [fudongbai](https://hackerone.com/fudongbai) for discovering
+and reporting this issue.
+
+Friendly reminder, we have a [bug bounty
+program](https://hackerone.com/tendermint).
+
+### SECURITY:
+
+- [p2p] [TODO](hxxp://githublink) Fix for panic on nil public key send to a peer
+
 ## v0.32.4
 
 *September 19, 2019*
@@ -22,9 +90,9 @@ program](https://hackerone.com/tendermint).
 - [deps] [\#3951](https://github.com/tendermint/tendermint/pull/3951) bump github.com/stretchr/testify from 1.3.0 to 1.4.0
 - [deps] [\#3945](https://github.com/tendermint/tendermint/pull/3945) bump github.com/gorilla/websocket from 1.2.0 to 1.4.1
 - [deps] [\#3948](https://github.com/tendermint/tendermint/pull/3948) bump github.com/libp2p/go-buffer-pool from 0.0.1 to 0.0.2
-- [deps] [\#3943](https://github.com/tendermint/tendermint/pull/3943) bump github.com/fortytw2/leaktest from 1.2.0 to 1.3.0 
-- [deps] [\#3939](https://github.com/tendermint/tendermint/pull/3939) bump github.com/rs/cors from 1.6.0 to 1.7.0 
-- [deps] [\#3937](https://github.com/tendermint/tendermint/pull/3937) bump github.com/magiconair/properties from 1.8.0 to 1.8.1 
+- [deps] [\#3943](https://github.com/tendermint/tendermint/pull/3943) bump github.com/fortytw2/leaktest from 1.2.0 to 1.3.0
+- [deps] [\#3939](https://github.com/tendermint/tendermint/pull/3939) bump github.com/rs/cors from 1.6.0 to 1.7.0
+- [deps] [\#3937](https://github.com/tendermint/tendermint/pull/3937) bump github.com/magiconair/properties from 1.8.0 to 1.8.1
 - [deps] [\#3947](https://github.com/tendermint/tendermint/pull/3947) update gogo/protobuf version from v1.2.1 to v1.3.0
 - [deps] [\#4001](https://github.com/tendermint/tendermint/pull/4001) bump github.com/tendermint/tm-db from 0.1.1 to 0.2.0
 

CHANGELOG_PENDING.md

@@ -1,4 +1,4 @@
-## v0.32.5
+## v0.32.8
 
 \*\*
 

consensus/reactor.go

@@ -1453,11 +1453,17 @@ func (m *NewValidBlockMessage) ValidateBasic() error {
 	if err := m.BlockPartsHeader.ValidateBasic(); err != nil {
 		return fmt.Errorf("Wrong BlockPartsHeader: %v", err)
 	}
+	if m.BlockParts.Size() == 0 {
+		return errors.New("Empty BlockParts")
+	}
 	if m.BlockParts.Size() != m.BlockPartsHeader.Total {
 		return fmt.Errorf("BlockParts bit array size %d not equal to BlockPartsHeader.Total %d",
 			m.BlockParts.Size(),
 			m.BlockPartsHeader.Total)
 	}
+	if m.BlockParts.Size() > types.MaxBlockPartsCount {
+		return errors.Errorf("BlockParts bit array is too big: %d, max: %d", m.BlockParts.Size(), types.MaxBlockPartsCount)
+	}
 	return nil
 }
 
@@ -1504,6 +1510,9 @@ func (m *ProposalPOLMessage) ValidateBasic() error {
 	if m.ProposalPOL.Size() == 0 {
 		return errors.New("Empty ProposalPOL bit array")
 	}
+	if m.ProposalPOL.Size() > types.MaxVotesCount {
+		return errors.Errorf("ProposalPOL bit array is too big: %d, max: %d", m.ProposalPOL.Size(), types.MaxVotesCount)
+	}
 	return nil
 }
 
@@ -1647,6 +1656,9 @@ func (m *VoteSetBitsMessage) ValidateBasic() error {
 		return fmt.Errorf("Wrong BlockID: %v", err)
 	}
 	// NOTE: Votes.Size() can be zero if the node does not have any
+	if m.Votes.Size() > types.MaxVotesCount {
+		return fmt.Errorf("Votes bit array is too big: %d, max: %d", m.Votes.Size(), types.MaxVotesCount)
+	}
 	return nil
 }
 

consensus/reactor_test.go

@@ -19,6 +19,7 @@ import (
 	abci "github.com/tendermint/tendermint/abci/types"
 	cfg "github.com/tendermint/tendermint/config"
 	cstypes "github.com/tendermint/tendermint/consensus/types"
+	"github.com/tendermint/tendermint/crypto/tmhash"
 	cmn "github.com/tendermint/tendermint/libs/common"
 	"github.com/tendermint/tendermint/libs/log"
 	mempl "github.com/tendermint/tendermint/mempool"
@@ -671,67 +672,82 @@ func TestNewRoundStepMessageValidateBasic(t *testing.T) {
 }
 
 func TestNewValidBlockMessageValidateBasic(t *testing.T) {
-	testBitArray := cmn.NewBitArray(1)
 	testCases := []struct {
-		testName          string
-		messageHeight     int64
-		messageRound      int
-		messageBlockParts *cmn.BitArray
-		expectErr         bool
+		malleateFn func(*NewValidBlockMessage)
+		expErr     string
 	}{
-		{"Valid Message", 0, 0, testBitArray, false},
-		{"Invalid Message", -1, 0, testBitArray, true},
-		{"Invalid Message", 0, -1, testBitArray, true},
-		{"Invalid Message", 0, 0, cmn.NewBitArray(0), true},
+		{func(msg *NewValidBlockMessage) {}, ""},
+		{func(msg *NewValidBlockMessage) { msg.Height = -1 }, "Negative Height"},
+		{func(msg *NewValidBlockMessage) { msg.Round = -1 }, "Negative Round"},
+		{
+			func(msg *NewValidBlockMessage) { msg.BlockPartsHeader.Total = 2 },
+			"BlockParts bit array size 1 not equal to BlockPartsHeader.Total 2",
+		},
+		{
+			func(msg *NewValidBlockMessage) { msg.BlockPartsHeader.Total = 0; msg.BlockParts = cmn.NewBitArray(0) },
+			"Empty BlockParts",
+		},
+		{
+			func(msg *NewValidBlockMessage) { msg.BlockParts = cmn.NewBitArray(types.MaxBlockPartsCount + 1) },
+			"BlockParts bit array size 1602 not equal to BlockPartsHeader.Total 1",
+		},
 	}
 
-	for _, tc := range testCases {
+	for i, tc := range testCases {
 		tc := tc
-		t.Run(tc.testName, func(t *testing.T) {
-			message := NewValidBlockMessage{
-				Height:     tc.messageHeight,
-				Round:      tc.messageRound,
-				BlockParts: tc.messageBlockParts,
+		t.Run(fmt.Sprintf("#%d", i), func(t *testing.T) {
+			msg := &NewValidBlockMessage{
+				Height: 1,
+				Round:  0,
+				BlockPartsHeader: types.PartSetHeader{
+					Total: 1,
+				},
+				BlockParts: cmn.NewBitArray(1),
 			}
 
-			message.BlockPartsHeader.Total = 1
-
-			assert.Equal(t, tc.expectErr, message.ValidateBasic() != nil, "Validate Basic had an unexpected result")
+			tc.malleateFn(msg)
+			err := msg.ValidateBasic()
+			if tc.expErr != "" && assert.Error(t, err) {
+				assert.Contains(t, err.Error(), tc.expErr)
+			}
 		})
 	}
 }
 
 func TestProposalPOLMessageValidateBasic(t *testing.T) {
-	testBitArray := cmn.NewBitArray(1)
 	testCases := []struct {
-		testName                string
-		messageHeight           int64
-		messageProposalPOLRound int
-		messageProposalPOL      *cmn.BitArray
-		expectErr               bool
+		malleateFn func(*ProposalPOLMessage)
+		expErr     string
 	}{
-		{"Valid Message", 0, 0, testBitArray, false},
-		{"Invalid Message", -1, 0, testBitArray, true},
-		{"Invalid Message", 0, -1, testBitArray, true},
-		{"Invalid Message", 0, 0, cmn.NewBitArray(0), true},
+		{func(msg *ProposalPOLMessage) {}, ""},
+		{func(msg *ProposalPOLMessage) { msg.Height = -1 }, "Negative Height"},
+		{func(msg *ProposalPOLMessage) { msg.ProposalPOLRound = -1 }, "Negative ProposalPOLRound"},
+		{func(msg *ProposalPOLMessage) { msg.ProposalPOL = cmn.NewBitArray(0) }, "Empty ProposalPOL bit array"},
+		{func(msg *ProposalPOLMessage) { msg.ProposalPOL = cmn.NewBitArray(types.MaxVotesCount + 1) },
+			"ProposalPOL bit array is too big: 10001, max: 10000"},
 	}
 
-	for _, tc := range testCases {
+	for i, tc := range testCases {
 		tc := tc
-		t.Run(tc.testName, func(t *testing.T) {
-			message := ProposalPOLMessage{
-				Height:           tc.messageHeight,
-				ProposalPOLRound: tc.messageProposalPOLRound,
-				ProposalPOL:      tc.messageProposalPOL,
+		t.Run(fmt.Sprintf("#%d", i), func(t *testing.T) {
+			msg := &ProposalPOLMessage{
+				Height:           1,
+				ProposalPOLRound: 1,
+				ProposalPOL:      cmn.NewBitArray(1),
 			}
 
-			assert.Equal(t, tc.expectErr, message.ValidateBasic() != nil, "Validate Basic had an unexpected result")
+			tc.malleateFn(msg)
+			err := msg.ValidateBasic()
+			if tc.expErr != "" && assert.Error(t, err) {
+				assert.Contains(t, err.Error(), tc.expErr)
+			}
 		})
 	}
 }
 
 func TestBlockPartMessageValidateBasic(t *testing.T) {
 	testPart := new(types.Part)
+	testPart.Proof.LeafHash = tmhash.Sum([]byte("leaf"))
 	testCases := []struct {
 		testName      string
 		messageHeight int64
@@ -845,49 +861,43 @@ func TestVoteSetMaj23MessageValidateBasic(t *testing.T) {
 }
 
 func TestVoteSetBitsMessageValidateBasic(t *testing.T) {
-	const (
-		validSignedMsgType   types.SignedMsgType = 0x01
-		invalidSignedMsgType types.SignedMsgType = 0x03
-	)
-
-	validBlockID := types.BlockID{}
-	invalidBlockID := types.BlockID{
-		Hash: cmn.HexBytes{},
-		PartsHeader: types.PartSetHeader{
-			Total: -1,
-			Hash:  cmn.HexBytes{},
-		},
-	}
-	testBitArray := cmn.NewBitArray(1)
-
-	testCases := []struct {
-		testName       string
-		messageHeight  int64
-		messageRound   int
-		messageType    types.SignedMsgType
-		messageBlockID types.BlockID
-		messageVotes   *cmn.BitArray
-		expectErr      bool
+	testCases := []struct { // nolint: maligned
+		malleateFn func(*VoteSetBitsMessage)
+		expErr     string
 	}{
-		{"Valid Message", 0, 0, validSignedMsgType, validBlockID, testBitArray, false},
-		{"Invalid Message", -1, 0, validSignedMsgType, validBlockID, testBitArray, true},
-		{"Invalid Message", 0, -1, validSignedMsgType, validBlockID, testBitArray, true},
-		{"Invalid Message", 0, 0, invalidSignedMsgType, validBlockID, testBitArray, true},
-		{"Invalid Message", 0, 0, validSignedMsgType, invalidBlockID, testBitArray, true},
+		{func(msg *VoteSetBitsMessage) {}, ""},
+		{func(msg *VoteSetBitsMessage) { msg.Height = -1 }, "Negative Height"},
+		{func(msg *VoteSetBitsMessage) { msg.Round = -1 }, "Negative Round"},
+		{func(msg *VoteSetBitsMessage) { msg.Type = 0x03 }, "Invalid Type"},
+		{func(msg *VoteSetBitsMessage) {
+			msg.BlockID = types.BlockID{
+				Hash: cmn.HexBytes{},
+				PartsHeader: types.PartSetHeader{
+					Total: -1,
+					Hash:  cmn.HexBytes{},
+				},
+			}
+		}, "Wrong BlockID: Wrong PartsHeader: Negative Total"},
+		{func(msg *VoteSetBitsMessage) { msg.Votes = cmn.NewBitArray(types.MaxVotesCount + 1) },
+			"Votes bit array is too big: 10001, max: 10000"},
 	}
 
-	for _, tc := range testCases {
+	for i, tc := range testCases {
 		tc := tc
-		t.Run(tc.testName, func(t *testing.T) {
-			message := VoteSetBitsMessage{
-				Height: tc.messageHeight,
-				Round:  tc.messageRound,
-				Type:   tc.messageType,
-				// Votes:   tc.messageVotes,
-				BlockID: tc.messageBlockID,
+		t.Run(fmt.Sprintf("#%d", i), func(t *testing.T) {
+			msg := &VoteSetBitsMessage{
+				Height:  1,
+				Round:   0,
+				Type:    0x01,
+				Votes:   cmn.NewBitArray(1),
+				BlockID: types.BlockID{},
 			}
 
-			assert.Equal(t, tc.expectErr, message.ValidateBasic() != nil, "Validate Basic had an unexpected result")
+			tc.malleateFn(msg)
+			err := msg.ValidateBasic()
+			if tc.expErr != "" && assert.Error(t, err) {
+				assert.Contains(t, err.Error(), tc.expErr)
+			}
 		})
 	}
 }

consensus/replay_test.go

@@ -228,15 +228,15 @@ func (e ReachedHeightToStopError) Error() string {
 
 // Write simulate WAL's crashing by sending an error to the panicCh and then
 // exiting the cs.receiveRoutine.
-func (w *crashingWAL) Write(m WALMessage) {
+func (w *crashingWAL) Write(m WALMessage) error {
 	if endMsg, ok := m.(EndHeightMessage); ok {
 		if endMsg.Height == w.heightToStop {
 			w.panicCh <- ReachedHeightToStopError{endMsg.Height}
 			runtime.Goexit()
-		} else {
-			w.next.Write(m)
+			return nil
 		}
-		return
+
+		return w.next.Write(m)
 	}
 
 	if w.msgIndex > w.lastPanickedForMsgIndex {
@@ -244,14 +244,15 @@ func (w *crashingWAL) Write(m WALMessage) {
 		_, file, line, _ := runtime.Caller(1)
 		w.panicCh <- WALWriteError{fmt.Sprintf("failed to write %T to WAL (fileline: %s:%d)", m, file, line)}
 		runtime.Goexit()
-	} else {
-		w.msgIndex++
-		w.next.Write(m)
+		return nil
 	}
+
+	w.msgIndex++
+	return w.next.Write(m)
 }
 
-func (w *crashingWAL) WriteSync(m WALMessage) {
-	w.Write(m)
+func (w *crashingWAL) WriteSync(m WALMessage) error {
+	return w.Write(m)
 }
 
 func (w *crashingWAL) FlushAndSync() error { return w.next.FlushAndSync() }

consensus/state.go

@@ -632,7 +632,10 @@ func (cs *ConsensusState) receiveRoutine(maxSteps int) {
 			// may generate internal events (votes, complete proposals, 2/3 majorities)
 			cs.handleMsg(mi)
 		case mi = <-cs.internalMsgQueue:
-			cs.wal.WriteSync(mi) // NOTE: fsync
+			err := cs.wal.WriteSync(mi) // NOTE: fsync
+			if err != nil {
+				panic(fmt.Sprintf("Failed to write %v msg to consensus wal due to %v. Check your FS and restart the node", mi, err))
+			}
 
 			if _, ok := mi.Msg.(*VoteMessage); ok {
 				// we actually want to simulate failing during
@@ -1329,7 +1332,10 @@ func (cs *ConsensusState) finalizeCommit(height int64) {
 	// Either way, the ConsensusState should not be resumed until we
 	// successfully call ApplyBlock (ie. later here, or in Handshake after
 	// restart).
-	cs.wal.WriteSync(EndHeightMessage{height}) // NOTE: fsync
+	endMsg := EndHeightMessage{height}
+	if err := cs.wal.WriteSync(endMsg); err != nil { // NOTE: fsync
+		panic(fmt.Sprintf("Failed to write %v msg to consensus wal due to %v. Check your FS and restart the node", endMsg, err))
+	}
 
 	fail.Fail() // XXX
 

consensus/wal.go

@@ -19,8 +19,8 @@ import (
 )
 
 const (
-	// must be greater than types.BlockPartSizeBytes + a few bytes
-	maxMsgSizeBytes = 1024 * 1024 // 1MB
+	// amino overhead + time.Time + max consensus msg size
+	maxMsgSizeBytes = maxMsgSize + 24
 
 	// how often the WAL should be sync'd during period sync'ing
 	walDefaultFlushInterval = 2 * time.Second
@@ -29,8 +29,9 @@ const (
 //--------------------------------------------------------
 // types and functions for savings consensus messages
 
+// TimedWALMessage wraps WALMessage and adds Time for debugging purposes.
 type TimedWALMessage struct {
-	Time time.Time  `json:"time"` // for debugging purposes
+	Time time.Time  `json:"time"`
 	Msg  WALMessage `json:"msg"`
 }
 
@@ -55,8 +56,8 @@ func RegisterWALMessages(cdc *amino.Codec) {
 
 // WAL is an interface for any write-ahead logger.
 type WAL interface {
-	Write(WALMessage)
-	WriteSync(WALMessage)
+	Write(WALMessage) error
+	WriteSync(WALMessage) error
 	FlushAndSync() error
 
 	SearchForEndHeight(height int64, options *WALSearchOptions) (rd io.ReadCloser, found bool, err error)
@@ -174,29 +175,39 @@ func (wal *baseWAL) Wait() {
 // Write is called in newStep and for each receive on the
 // peerMsgQueue and the timeoutTicker.
 // NOTE: does not call fsync()
-func (wal *baseWAL) Write(msg WALMessage) {
+func (wal *baseWAL) Write(msg WALMessage) error {
 	if wal == nil {
-		return
+		return nil
 	}
 
-	// Write the wal message
 	if err := wal.enc.Encode(&TimedWALMessage{tmtime.Now(), msg}); err != nil {
-		panic(fmt.Sprintf("Error writing msg to consensus wal: %v \n\nMessage: %v", err, msg))
+		wal.Logger.Error("Error writing msg to consensus wal. WARNING: recover may not be possible for the current height",
+			"err", err, "msg", msg)
+		return err
 	}
+
+	return nil
 }
 
 // WriteSync is called when we receive a msg from ourselves
 // so that we write to disk before sending signed messages.
 // NOTE: calls fsync()
-func (wal *baseWAL) WriteSync(msg WALMessage) {
+func (wal *baseWAL) WriteSync(msg WALMessage) error {
 	if wal == nil {
-		return
+		return nil
 	}
 
-	wal.Write(msg)
-	if err := wal.FlushAndSync(); err != nil {
-		panic(fmt.Sprintf("Error flushing consensus wal buf to file. Error: %v \n", err))
+	if err := wal.Write(msg); err != nil {
+		return err
 	}
+
+	if err := wal.FlushAndSync(); err != nil {
+		wal.Logger.Error("WriteSync failed to flush consensus wal. WARNING: may result in creating alternative proposals / votes for the current height iff the node restarted",
+			"err", err)
+		return err
+	}
+
+	return nil
 }
 
 // WALSearchOptions are optional arguments to SearchForEndHeight.
@@ -285,7 +296,7 @@ func (enc *WALEncoder) Encode(v *TimedWALMessage) error {
 	crc := crc32.Checksum(data, crc32c)
 	length := uint32(len(data))
 	if length > maxMsgSizeBytes {
-		return fmt.Errorf("Msg is too big: %d bytes, max: %d bytes", length, maxMsgSizeBytes)
+		return fmt.Errorf("msg is too big: %d bytes, max: %d bytes", length, maxMsgSizeBytes)
 	}
 	totalLength := 8 + int(length)
 
@@ -295,7 +306,6 @@ func (enc *WALEncoder) Encode(v *TimedWALMessage) error {
 	copy(msg[8:], data)
 
 	_, err := enc.wr.Write(msg)
-
 	return err
 }
 
@@ -383,9 +393,9 @@ type nilWAL struct{}
 
 var _ WAL = nilWAL{}
 
-func (nilWAL) Write(m WALMessage)     {}
-func (nilWAL) WriteSync(m WALMessage) {}
-func (nilWAL) FlushAndSync() error    { return nil }
+func (nilWAL) Write(m WALMessage) error     { return nil }
+func (nilWAL) WriteSync(m WALMessage) error { return nil }
+func (nilWAL) FlushAndSync() error          { return nil }
 func (nilWAL) SearchForEndHeight(height int64, options *WALSearchOptions) (rd io.ReadCloser, found bool, err error) {
 	return nil, false, nil
 }

consensus/wal_generator.go

@@ -168,10 +168,10 @@ func newByteBufferWAL(logger log.Logger, enc *WALEncoder, nBlocks int64, signalS
 // Save writes message to the internal buffer except when heightToStop is
 // reached, in which case it will signal the caller via signalWhenStopsTo and
 // skip writing.
-func (w *byteBufferWAL) Write(m WALMessage) {
+func (w *byteBufferWAL) Write(m WALMessage) error {
 	if w.stopped {
 		w.logger.Debug("WAL already stopped. Not writing message", "msg", m)
-		return
+		return nil
 	}
 
 	if endMsg, ok := m.(EndHeightMessage); ok {
@@ -180,7 +180,7 @@ func (w *byteBufferWAL) Write(m WALMessage) {
 			w.logger.Debug("Stopping WAL at height", "height", endMsg.Height)
 			w.signalWhenStopsTo <- struct{}{}
 			w.stopped = true
-			return
+			return nil
 		}
 	}
 
@@ -189,10 +189,12 @@ func (w *byteBufferWAL) Write(m WALMessage) {
 	if err != nil {
 		panic(fmt.Sprintf("failed to encode the msg %v", m))
 	}
+
+	return nil
 }
 
-func (w *byteBufferWAL) WriteSync(m WALMessage) {
-	w.Write(m)
+func (w *byteBufferWAL) WriteSync(m WALMessage) error {
+	return w.Write(m)
 }
 
 func (w *byteBufferWAL) FlushAndSync() error { return nil }

consensus/wal_test.go

@@ -11,14 +11,15 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	"github.com/tendermint/tendermint/consensus/types"
+	"github.com/tendermint/tendermint/crypto/merkle"
 	"github.com/tendermint/tendermint/libs/autofile"
 	"github.com/tendermint/tendermint/libs/log"
 	tmtypes "github.com/tendermint/tendermint/types"
 	tmtime "github.com/tendermint/tendermint/types/time"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 )
 
 const (
@@ -103,7 +104,7 @@ func TestWALEncoderDecoder(t *testing.T) {
 	}
 }
 
-func TestWALWritePanicsIfMsgIsTooBig(t *testing.T) {
+func TestWALWrite(t *testing.T) {
 	walDir, err := ioutil.TempDir("", "wal")
 	require.NoError(t, err)
 	defer os.RemoveAll(walDir)
@@ -120,7 +121,24 @@ func TestWALWritePanicsIfMsgIsTooBig(t *testing.T) {
 		wal.Wait()
 	}()
 
-	assert.Panics(t, func() { wal.Write(make([]byte, maxMsgSizeBytes+1)) })
+	// 1) Write returns an error if msg is too big
+	msg := &BlockPartMessage{
+		Height: 1,
+		Round:  1,
+		Part: &tmtypes.Part{
+			Index: 1,
+			Bytes: make([]byte, 1),
+			Proof: merkle.SimpleProof{
+				Total:    1,
+				Index:    1,
+				LeafHash: make([]byte, maxMsgSizeBytes-30),
+			},
+		},
+	}
+	err = wal.Write(msg)
+	if assert.Error(t, err) {
+		assert.Contains(t, err.Error(), "msg is too big")
+	}
 }
 
 func TestWALSearchForEndHeight(t *testing.T) {

crypto/merkle/simple_proof.go

@@ -5,6 +5,11 @@ import (
 	"fmt"
 
 	"github.com/pkg/errors"
+	"github.com/tendermint/tendermint/crypto/tmhash"
+)
+
+const (
+	maxAunts = 100
 )
 
 // SimpleProof represents a simple Merkle proof.
@@ -108,6 +113,30 @@ func (sp *SimpleProof) StringIndented(indent string) string {
 		indent)
 }
 
+// ValidateBasic performs basic validation.
+// NOTE: - it expects LeafHash and Aunts of tmhash.Size size
+//			 - it expects no more than 100 aunts
+func (sp *SimpleProof) ValidateBasic() error {
+	if sp.Total < 0 {
+		return errors.New("negative Total")
+	}
+	if sp.Index < 0 {
+		return errors.New("negative Index")
+	}
+	if len(sp.LeafHash) != tmhash.Size {
+		return errors.Errorf("expected LeafHash size to be %d, got %d", tmhash.Size, len(sp.LeafHash))
+	}
+	if len(sp.Aunts) > maxAunts {
+		return errors.Errorf("expected no more than %d aunts, got %d", maxAunts, len(sp.Aunts))
+	}
+	for i, auntHash := range sp.Aunts {
+		if len(auntHash) != tmhash.Size {
+			return errors.Errorf("expected Aunts#%d size to be %d, got %d", i, tmhash.Size, len(auntHash))
+		}
+	}
+	return nil
+}
+
 // Use the leafHash and innerHashes to get the root merkle hash.
 // If the length of the innerHashes slice isn't exactly correct, the result is nil.
 // Recursive impl.

crypto/merkle/simple_proof_test.go

@@ -0,0 +1,38 @@
+package merkle
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSimpleProofValidateBasic(t *testing.T) {
+	testCases := []struct {
+		testName      string
+		malleateProof func(*SimpleProof)
+		errStr        string
+	}{
+		{"Good", func(sp *SimpleProof) {}, ""},
+		{"Negative Total", func(sp *SimpleProof) { sp.Total = -1 }, "negative Total"},
+		{"Negative Index", func(sp *SimpleProof) { sp.Index = -1 }, "negative Index"},
+		{"Invalid LeafHash", func(sp *SimpleProof) { sp.LeafHash = make([]byte, 10) }, "expected LeafHash size to be 32, got 10"},
+		{"Too many Aunts", func(sp *SimpleProof) { sp.Aunts = make([][]byte, maxAunts+1) }, "expected no more than 100 aunts, got 101"},
+		{"Invalid Aunt", func(sp *SimpleProof) { sp.Aunts[0] = make([]byte, 10) }, "expected Aunts#0 size to be 32, got 10"},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.testName, func(t *testing.T) {
+			_, proofs := SimpleProofsFromByteSlices([][]byte{
+				[]byte("apple"),
+				[]byte("watermelon"),
+				[]byte("kiwi"),
+			})
+			tc.malleateProof(proofs[0])
+			err := proofs[0].ValidateBasic()
+			if tc.errStr != "" {
+				assert.Contains(t, err.Error(), tc.errStr)
+			}
+		})
+	}
+}

crypto/multisig/threshold_pubkey.go

@@ -21,6 +21,11 @@ func NewPubKeyMultisigThreshold(k int, pubkeys []crypto.PubKey) crypto.PubKey {
 	if len(pubkeys) < k {
 		panic("threshold k of n multisignature: len(pubkeys) < k")
 	}
+	for _, pubkey := range pubkeys {
+		if pubkey == nil {
+			panic("nil pubkey")
+		}
+	}
 	return PubKeyMultisigThreshold{uint(k), pubkeys}
 }
 

docs/spec/blockchain/encoding.md

@@ -288,6 +288,8 @@ func computeHashFromAunts(index, total int, leafHash []byte, innerHashes [][]byt
 }
 ```
 
+The number of aunts is limited to 100 (`maxAunts`) to protect the node against DOS attacks.
+
 ### IAVL+ Tree
 
 Because Tendermint only uses a Simple Merkle Tree, application developers are expect to use their own Merkle tree in their applications. For example, the IAVL+ Tree - an immutable self-balancing binary tree for persisting application state is used by the [Cosmos SDK](https://github.com/cosmos/cosmos-sdk/blob/master/docs/clients/lite/specification.md)

docs/spec/reactors/consensus/consensus-reactor.md

@@ -96,11 +96,13 @@ type PeerRoundState struct {
 
 ## Receive method of Consensus reactor
 
-The entry point of the Consensus reactor is a receive method. When a message is received from a peer p,
-normally the peer round state is updated correspondingly, and some messages
-are passed for further processing, for example to ConsensusState service. We now specify the processing of messages
-in the receive method of Consensus reactor for each message type. In the following message handler, `rs` and `prs` denote
-`RoundState` and `PeerRoundState`, respectively.
+The entry point of the Consensus reactor is a receive method. When a message is
+received from a peer p, normally the peer round state is updated
+correspondingly, and some messages are passed for further processing, for
+example to ConsensusState service. We now specify the processing of messages in
+the receive method of Consensus reactor for each message type. In the following
+message handler, `rs` and `prs` denote `RoundState` and `PeerRoundState`,
+respectively.
 
 ### NewRoundStepMessage handler
 
@@ -134,13 +136,16 @@ handleMessage(msg):
 ```
 handleMessage(msg):
     if prs.Height != msg.Height then return
-    
+
     if prs.Round != msg.Round && !msg.IsCommit then return
-    
+
     prs.ProposalBlockPartsHeader = msg.BlockPartsHeader
     prs.ProposalBlockParts = msg.BlockParts
 ```
 
+The number of block parts is limited to 1601 (`types.MaxBlockPartsCount`) to
+protect the node against DOS attacks.
+
 ### HasVoteMessage handler
 
 ```
@@ -179,6 +184,9 @@ handleMessage(msg):
     prs.ProposalPOL = msg.ProposalPOL
 ```
 
+The number of votes is limited to 10000 (`types.MaxVotesCount`) to protect the
+node against DOS attacks.
+
 ### BlockPartMessage handler
 
 ```
@@ -203,6 +211,9 @@ handleMessage(msg):
     Update prs for the bit-array of votes peer claims to have for the msg.BlockID
 ```
 
+The number of votes is limited to 10000 (`types.MaxVotesCount`) to protect the
+node against DOS attacks.
+
 ## Gossip Data Routine
 
 It is used to send the following messages to the peer: `BlockPartMessage`, `ProposalMessage` and
@@ -338,7 +349,7 @@ BlockID has seen +2/3 votes. This routine is based on the local RoundState (`rs`
 
 ## Broadcast routine
 
-The Broadcast routine subscribes to an internal event bus to receive new round steps and votes messages, and broadcasts messages to peers upon receiving those 
+The Broadcast routine subscribes to an internal event bus to receive new round steps and votes messages, and broadcasts messages to peers upon receiving those
 events.
 It broadcasts `NewRoundStepMessage` or `CommitStepMessage` upon new round state event. Note that
 broadcasting these messages does not depend on the PeerRoundState; it is sent on the StateChannel.

p2p/conn/secret_connection.go

@@ -7,21 +7,22 @@ import (
 	"crypto/sha256"
 	"crypto/subtle"
 	"encoding/binary"
-	"errors"
 	"io"
 	"math"
 	"net"
 	"sync"
 	"time"
 
+	pool "github.com/libp2p/go-buffer-pool"
+	"github.com/pkg/errors"
 	"golang.org/x/crypto/chacha20poly1305"
 	"golang.org/x/crypto/curve25519"
+	"golang.org/x/crypto/hkdf"
 	"golang.org/x/crypto/nacl/box"
 
-	pool "github.com/libp2p/go-buffer-pool"
 	"github.com/tendermint/tendermint/crypto"
+	"github.com/tendermint/tendermint/crypto/ed25519"
 	cmn "github.com/tendermint/tendermint/libs/common"
-	"golang.org/x/crypto/hkdf"
 )
 
 // 4 + 1024 == 1028 total frame size
@@ -107,11 +108,11 @@ func MakeSecretConnection(conn io.ReadWriteCloser, locPrivKey crypto.PrivKey) (*
 
 	sendAead, err := chacha20poly1305.New(sendSecret[:])
 	if err != nil {
-		return nil, errors.New("Invalid send SecretConnection Key")
+		return nil, errors.New("invalid send SecretConnection Key")
 	}
 	recvAead, err := chacha20poly1305.New(recvSecret[:])
 	if err != nil {
-		return nil, errors.New("Invalid receive SecretConnection Key")
+		return nil, errors.New("invalid receive SecretConnection Key")
 	}
 	// Construct SecretConnection.
 	sc := &SecretConnection{
@@ -133,8 +134,13 @@ func MakeSecretConnection(conn io.ReadWriteCloser, locPrivKey crypto.PrivKey) (*
 	}
 
 	remPubKey, remSignature := authSigMsg.Key, authSigMsg.Sig
+
+	if _, ok := remPubKey.(ed25519.PubKeyEd25519); !ok {
+		return nil, errors.Errorf("expected ed25519 pubkey, got %T", remPubKey)
+	}
+
 	if !remPubKey.VerifyBytes(challenge[:], remSignature) {
-		return nil, errors.New("Challenge verification failed")
+		return nil, errors.New("challenge verification failed")
 	}
 
 	// We've authorized.
@@ -217,7 +223,7 @@ func (sc *SecretConnection) Read(data []byte) (n int, err error) {
 	defer pool.Put(frame)
 	_, err = sc.recvAead.Open(frame[:0], sc.recvNonce[:], sealedFrame, nil)
 	if err != nil {
-		return n, errors.New("Failed to decrypt SecretConnection")
+		return n, errors.New("failed to decrypt SecretConnection")
 	}
 	incrNonce(sc.recvNonce)
 	// end decryption

p2p/conn/secret_connection_test.go

@@ -17,7 +17,9 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"github.com/tendermint/tendermint/crypto"
 	"github.com/tendermint/tendermint/crypto/ed25519"
+	"github.com/tendermint/tendermint/crypto/secp256k1"
 	cmn "github.com/tendermint/tendermint/libs/common"
 )
 
@@ -366,6 +368,51 @@ func TestDeriveSecretsAndChallengeGolden(t *testing.T) {
 	}
 }
 
+type privKeyWithNilPubKey struct {
+	orig crypto.PrivKey
+}
+
+func (pk privKeyWithNilPubKey) Bytes() []byte                   { return pk.orig.Bytes() }
+func (pk privKeyWithNilPubKey) Sign(msg []byte) ([]byte, error) { return pk.orig.Sign(msg) }
+func (pk privKeyWithNilPubKey) PubKey() crypto.PubKey           { return nil }
+func (pk privKeyWithNilPubKey) Equals(pk2 crypto.PrivKey) bool  { return pk.orig.Equals(pk2) }
+
+func TestNilPubkey(t *testing.T) {
+	var fooConn, barConn = makeKVStoreConnPair()
+	var fooPrvKey = ed25519.GenPrivKey()
+	var barPrvKey = privKeyWithNilPubKey{ed25519.GenPrivKey()}
+
+	go func() {
+		_, err := MakeSecretConnection(barConn, barPrvKey)
+		assert.NoError(t, err)
+	}()
+
+	assert.NotPanics(t, func() {
+		_, err := MakeSecretConnection(fooConn, fooPrvKey)
+		if assert.Error(t, err) {
+			assert.Equal(t, "expected ed25519 pubkey, got <nil>", err.Error())
+		}
+	})
+}
+
+func TestNonEd25519Pubkey(t *testing.T) {
+	var fooConn, barConn = makeKVStoreConnPair()
+	var fooPrvKey = ed25519.GenPrivKey()
+	var barPrvKey = secp256k1.GenPrivKey()
+
+	go func() {
+		_, err := MakeSecretConnection(barConn, barPrvKey)
+		assert.NoError(t, err)
+	}()
+
+	assert.NotPanics(t, func() {
+		_, err := MakeSecretConnection(fooConn, fooPrvKey)
+		if assert.Error(t, err) {
+			assert.Equal(t, "expected ed25519 pubkey, got secp256k1.PubKeySecp256k1", err.Error())
+		}
+	})
+}
+
 // Creates the data for a test vector file.
 // The file format is:
 // Hex(diffie_hellman_secret), loc_is_least, Hex(recvSecret), Hex(sendSecret), Hex(challenge)

p2p/transport.go

@@ -6,6 +6,8 @@ import (
 	"net"
 	"time"
 
+	"github.com/pkg/errors"
+
 	"github.com/tendermint/tendermint/crypto"
 	"github.com/tendermint/tendermint/p2p/conn"
 )
@@ -270,6 +272,23 @@ func (mt *MultiplexTransport) acceptPeers() {
 		//
 		// [0] https://en.wikipedia.org/wiki/Head-of-line_blocking
 		go func(c net.Conn) {
+			defer func() {
+				if r := recover(); r != nil {
+					err := ErrRejected{
+						conn:          c,
+						err:           errors.Errorf("recovered from panic: %v", r),
+						isAuthFailure: true,
+					}
+					select {
+					case mt.acceptc <- accept{err: err}:
+					case <-mt.closec:
+						// Give up if the transport was closed.
+						_ = c.Close()
+						return
+					}
+				}
+			}()
+
 			var (
 				nodeInfo   NodeInfo
 				secretConn *conn.SecretConnection

scripts/wal2json/main.go

@@ -56,8 +56,8 @@ func main() {
 			_, err = os.Stdout.Write([]byte("\n"))
 		}
 		if err == nil {
-			if end, ok := msg.Msg.(cs.EndHeightMessage); ok {
-				_, err = os.Stdout.Write([]byte(fmt.Sprintf("ENDHEIGHT %d\n", end.Height))) // nolint: errcheck, gas
+			if endMsg, ok := msg.Msg.(cs.EndHeightMessage); ok {
+				_, err = os.Stdout.Write([]byte(fmt.Sprintf("ENDHEIGHT %d\n", endMsg.Height))) // nolint: errcheck, gas
 			}
 		}
 		if err != nil {

types/params.go

@@ -14,6 +14,9 @@ const (
 
 	// BlockPartSizeBytes is the size of one block part.
 	BlockPartSizeBytes = 65536 // 64kB
+
+	// MaxBlockPartsCount is the maximum count of block parts.
+	MaxBlockPartsCount = (MaxBlockSizeBytes / BlockPartSizeBytes) + 1
 )
 
 // ConsensusParams contains consensus critical parameters that determine the

types/part_set.go

@@ -26,10 +26,13 @@ type Part struct {
 // ValidateBasic performs basic validation.
 func (part *Part) ValidateBasic() error {
 	if part.Index < 0 {
-		return errors.New("Negative Index")
+		return errors.New("negative Index")
 	}
 	if len(part.Bytes) > BlockPartSizeBytes {
-		return fmt.Errorf("Too big (max: %d)", BlockPartSizeBytes)
+		return errors.Errorf("too big: %d bytes, max: %d", len(part.Bytes), BlockPartSizeBytes)
+	}
+	if err := part.Proof.ValidateBasic(); err != nil {
+		return errors.Wrap(err, "wrong Proof")
 	}
 	return nil
 }

types/part_set_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/tendermint/tendermint/crypto/merkle"
 	cmn "github.com/tendermint/tendermint/libs/common"
 )
 
@@ -115,6 +116,13 @@ func TestPartValidateBasic(t *testing.T) {
 		{"Good Part", func(pt *Part) {}, false},
 		{"Negative index", func(pt *Part) { pt.Index = -1 }, true},
 		{"Too big part", func(pt *Part) { pt.Bytes = make([]byte, BlockPartSizeBytes+1) }, true},
+		{"Too big proof", func(pt *Part) {
+			pt.Proof = merkle.SimpleProof{
+				Total:    1,
+				Index:    1,
+				LeafHash: make([]byte, 1024*1024),
+			}
+		}, true},
 	}
 
 	for _, tc := range testCases {

types/vote_set.go

@@ -11,6 +11,12 @@ import (
 	cmn "github.com/tendermint/tendermint/libs/common"
 )
 
+const (
+	// MaxVotesCount is the maximum votes count. Used in ValidateBasic funcs for
+	// protection against DOS attacks.
+	MaxVotesCount = 10000
+)
+
 // UNSTABLE
 // XXX: duplicate of p2p.ID to avoid dependence between packages.
 // Perhaps we can have a minimal types package containing this (and other things?)

version/version.go

@@ -20,7 +20,7 @@ const (
 	// Must be a string because scripts like dist.sh read this file.
 	// XXX: Don't change the name of this variable or you will break
 	// automation :)
-	TMCoreSemVer = "0.32.4"
+	TMCoreSemVer = "0.32.7"
 
 	// ABCISemVer is the semantic version of the ABCI library
 	ABCISemVer  = "0.16.1"