diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 79d66bc05..53bef9054 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -39,6 +39,7 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - [all] [\#4608](https://github.com/tendermint/tendermint/pull/4608) Give reactors descriptive names when they're initialized - [lite2] [\#4575](https://github.com/tendermint/tendermint/pull/4575) Use bisection for within-range verification (@cmwaters) - [tools] \#4615 Allow developers to use Docker to generate proto stubs, via `make proto-gen-docker`. +- [p2p] \#4621(https://github.com/tendermint/tendermint/pull/4621) ban peers when messages are unsolicited or too frequent (@cmwaters) ### BUG FIXES: diff --git a/p2p/pex/errors.go b/p2p/pex/errors.go index 1fc54ea50..8f51d4217 100644 --- a/p2p/pex/errors.go +++ b/p2p/pex/errors.go @@ -1,6 +1,7 @@ package pex import ( + "errors" "fmt" "github.com/tendermint/tendermint/p2p" @@ -72,3 +73,6 @@ type ErrAddressBanned struct { func (err ErrAddressBanned) Error() string { return fmt.Sprintf("Address: %v is currently banned", err.Addr) } + +// ErrUnsolicitedList is thrown when a peer provides a list of addresses that have not been asked for. +var ErrUnsolicitedList = errors.New("unsolicited pexAddrsMessage") diff --git a/p2p/pex/pex_reactor.go b/p2p/pex/pex_reactor.go index 192e75fa1..d06814195 100644 --- a/p2p/pex/pex_reactor.go +++ b/p2p/pex/pex_reactor.go @@ -276,6 +276,7 @@ func (r *Reactor) Receive(chID byte, src Peer, msgBytes []byte) { // Check we're not receiving requests too frequently. if err := r.receiveRequest(src); err != nil { r.Switch.StopPeerForError(src, err) + r.book.MarkBad(src.SocketAddr(), defaultBanTime) return } r.SendAddrs(src, r.book.GetSelection()) @@ -285,6 +286,9 @@ func (r *Reactor) Receive(chID byte, src Peer, msgBytes []byte) { // If we asked for addresses, add them to the book if err := r.ReceiveAddrs(msg.Addrs, src); err != nil { r.Switch.StopPeerForError(src, err) + if err == ErrUnsolicitedList { + r.book.MarkBad(src.SocketAddr(), defaultBanTime) + } return } default: @@ -344,7 +348,7 @@ func (r *Reactor) RequestAddrs(p Peer) { func (r *Reactor) ReceiveAddrs(addrs []*p2p.NetAddress, src Peer) error { id := string(src.ID()) if !r.requestsSent.Has(id) { - return errors.New("unsolicited pexAddrsMessage") + return ErrUnsolicitedList } r.requestsSent.Delete(id) diff --git a/p2p/pex/pex_reactor_test.go b/p2p/pex/pex_reactor_test.go index 4cddf6352..edaf732ca 100644 --- a/p2p/pex/pex_reactor_test.go +++ b/p2p/pex/pex_reactor_test.go @@ -144,8 +144,11 @@ func TestPEXReactorRequestMessageAbuse(t *testing.T) { sw.SetAddrBook(book) peer := mock.NewPeer(nil) + peerAddr := peer.SocketAddr() p2p.AddPeerToSwitchPeerSet(sw, peer) assert.True(t, sw.Peers().Has(peer.ID())) + book.AddAddress(peerAddr, peerAddr) + require.True(t, book.HasAddress(peerAddr)) id := string(peer.ID()) msg := cdc.MustMarshalBinaryBare(&pexRequestMessage{}) @@ -164,6 +167,7 @@ func TestPEXReactorRequestMessageAbuse(t *testing.T) { r.Receive(PexChannel, peer, msg) assert.False(t, r.lastReceivedRequests.Has(id)) assert.False(t, sw.Peers().Has(peer.ID())) + assert.True(t, book.IsBanned(peerAddr)) } func TestPEXReactorAddrsMessageAbuse(t *testing.T) { @@ -192,9 +196,10 @@ func TestPEXReactorAddrsMessageAbuse(t *testing.T) { assert.False(t, r.requestsSent.Has(id)) assert.True(t, sw.Peers().Has(peer.ID())) - // receiving more addrs causes a disconnect + // receiving more unsolicited addrs causes a disconnect and ban r.Receive(PexChannel, peer, msg) assert.False(t, sw.Peers().Has(peer.ID())) + assert.True(t, book.IsBanned(peer.SocketAddr())) } func TestCheckSeeds(t *testing.T) {