p2p: PEX message abuse should ban as well as disconnect (#4621)

* mark unsolicited and too frequent messaged as bad

* add tests

* update changelog and fix error

* revised error types

Co-authored-by: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

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:
 

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")

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)
 

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) {