diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d34ae06e..6c3776935 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,32 @@ # 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* @@ -246,6 +273,33 @@ program](https://hackerone.com/tendermint). - [node] [\#3716](https://github.com/tendermint/tendermint/issues/3716) Fix a bug where `nil` is recorded as node's address - [node] [\#3741](https://github.com/tendermint/tendermint/issues/3741) Fix profiler blocking the entire node +## v0.31.11 + +*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.31.10 *October 8, 2019* diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 5a5cb7095..58ac79bfd 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -1,4 +1,4 @@ -## v0.32.7 +## v0.32.8 \*\* diff --git a/consensus/reactor.go b/consensus/reactor.go index 781f9d011..6d4b61219 100644 --- a/consensus/reactor.go +++ b/consensus/reactor.go @@ -1468,11 +1468,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 } @@ -1519,6 +1525,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 } @@ -1662,6 +1671,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 } diff --git a/consensus/reactor_test.go b/consensus/reactor_test.go index bc2d4cd16..94190837d 100644 --- a/consensus/reactor_test.go +++ b/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" @@ -698,67 +699,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 @@ -872,49 +888,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 { // nolint: maligned - expectErr bool - messageRound int - messageHeight int64 - testName string - messageType types.SignedMsgType - messageBlockID types.BlockID - messageVotes *cmn.BitArray + malleateFn func(*VoteSetBitsMessage) + expErr string }{ - {false, 0, 0, "Valid Message", validSignedMsgType, validBlockID, testBitArray}, - {true, -1, 0, "Invalid Message", validSignedMsgType, validBlockID, testBitArray}, - {true, 0, -1, "Invalid Message", validSignedMsgType, validBlockID, testBitArray}, - {true, 0, 0, "Invalid Message", invalidSignedMsgType, validBlockID, testBitArray}, - {true, 0, 0, "Invalid Message", validSignedMsgType, invalidBlockID, testBitArray}, + {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) + } }) } } diff --git a/consensus/replay_test.go b/consensus/replay_test.go index ae8e44fd2..36aba9e72 100644 --- a/consensus/replay_test.go +++ b/consensus/replay_test.go @@ -238,15 +238,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 { @@ -254,14 +254,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() } diff --git a/consensus/state.go b/consensus/state.go index a3e5cc52c..531a295ba 100644 --- a/consensus/state.go +++ b/consensus/state.go @@ -641,7 +641,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 @@ -1406,7 +1409,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 diff --git a/consensus/wal.go b/consensus/wal.go index 2bee286bd..d6bc7fa7b 100644 --- a/consensus/wal.go +++ b/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 + } + + if err := wal.Write(msg); err != nil { + return err } - wal.Write(msg) if err := wal.FlushAndSync(); err != nil { - panic(fmt.Sprintf("Error flushing consensus wal buf to file. Error: %v \n", err)) + 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. @@ -287,7 +298,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) @@ -297,7 +308,6 @@ func (enc *WALEncoder) Encode(v *TimedWALMessage) error { copy(msg[8:], data) _, err := enc.wr.Write(msg) - return err } @@ -388,9 +398,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 } diff --git a/consensus/wal_generator.go b/consensus/wal_generator.go index 82e531074..6ceeb19b6 100644 --- a/consensus/wal_generator.go +++ b/consensus/wal_generator.go @@ -169,10 +169,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 { @@ -181,7 +181,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 } } @@ -190,10 +190,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 } diff --git a/consensus/wal_test.go b/consensus/wal_test.go index 82d912f3a..6871f534d 100644 --- a/consensus/wal_test.go +++ b/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) { diff --git a/crypto/merkle/simple_proof.go b/crypto/merkle/simple_proof.go index da32157db..93b6f9fea 100644 --- a/crypto/merkle/simple_proof.go +++ b/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. diff --git a/crypto/merkle/simple_proof_test.go b/crypto/merkle/simple_proof_test.go new file mode 100644 index 000000000..1a517905b --- /dev/null +++ b/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) + } + }) + } +} diff --git a/docs/spec/blockchain/encoding.md b/docs/spec/blockchain/encoding.md index 170e91605..a171020b0 100644 --- a/docs/spec/blockchain/encoding.md +++ b/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) diff --git a/docs/spec/reactors/consensus/consensus-reactor.md b/docs/spec/reactors/consensus/consensus-reactor.md index 47c6949a7..0f74c5f88 100644 --- a/docs/spec/reactors/consensus/consensus-reactor.md +++ b/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. diff --git a/scripts/wal2json/main.go b/scripts/wal2json/main.go index ee90ecaa4..96aadd146 100644 --- a/scripts/wal2json/main.go +++ b/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 { diff --git a/types/params.go b/types/params.go index c9ab4aaf7..61c1d2da6 100644 --- a/types/params.go +++ b/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 diff --git a/types/part_set.go b/types/part_set.go index 389db7a0b..ecac027f9 100644 --- a/types/part_set.go +++ b/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 } diff --git a/types/part_set_test.go b/types/part_set_test.go index 37aacea75..706c8cae4 100644 --- a/types/part_set_test.go +++ b/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 { diff --git a/types/vote_set.go b/types/vote_set.go index 4f50c8049..028065f7c 100644 --- a/types/vote_set.go +++ b/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?) diff --git a/version/version.go b/version/version.go index 42ec287ab..35d4516d2 100644 --- a/version/version.go +++ b/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.6" + TMCoreSemVer = "0.32.7" // ABCISemVer is the semantic version of the ABCI library ABCISemVer = "0.16.1"