CVE-2020-5303: Out-of-bounds Write (CWE-787)
value: Tendermint before versions 0.33.3, 0.32.10, and 0.31.12 has a denial-of-service vulnerability. Tendermint does not limit the number of P2P connection requests. For each p2p connection, it allocates XXX bytes. Even though this memory is garbage collected once the connection is terminated (due to duplicate IP or reaching a maximum number of inbound peers), temporary memory spikes can lead to OOM (Out-Of-Memory) exceptions. Additionally, Tendermint does not reclaim activeID of a peer after it's removed in Mempool reactor. This does not happen all the time. It only happens when a connection fails (for any reason) before the Peer is created and added to all reactors. RemovePeer is therefore called before AddPeer, which leads to always growing memory (activeIDs map). The activeIDs map has a maximum size of 65535 and the node will panic if this map reaches the maximum. An attacker can create a lot of connection attempts (exploiting above denial of service), which ultimately will lead to the node panicking. These issues are patched in Tendermint 0.33.3 and 0.32.10.
CWE description: The product writes data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 3.7
Published: 2020-04-10T19:15Z
Repository: https://github.com/tendermint/tendermint
Language: Go
Files changed: node/node.go, p2p/transport.go, p2p/transport_test.go
Commit date: 2020-04-09 19:32:00 +0200
Commit message: p2p: limit the number of incoming connections

to p2p.max_num_inbound_peers + len(p2p.unconditional_peer_ids)

Diff: diff --git a/node/node.go b/node/node.go
index 70e725d42..5fb0664ea 100644
--- a/node/node.go
+++ b/node/node.go
@@ -469,10 +469,15 @@ func createTransport(
 			},
 		)
 	}
 
 	p2p.MultiplexTransportConnFilters(connFilters...)(transport)
+
+	// Limit the number of incoming connections.
+	max := config.P2P.MaxNumInboundPeers + len(splitAndTrimEmpty(config.P2P.UnconditionalPeerIDs, ",", " "))
+	p2p.MultiplexTransportMaxIncomingConnections(max)(transport)
+
 	return transport, peerFilters
 }
 
 func createSwitch(config *cfg.Config,
 	transport p2p.Transport,
diff --git a/p2p/transport.go b/p2p/transport.go
index 89d63da7b..6b749c61f 100644
--- a/p2p/transport.go
+++ b/p2p/transport.go
@@ -5,10 +5,11 @@ import (
 	"fmt"
 	"net"
 	"time"
 
 	"github.com/pkg/errors"
+	"golang.org/x/net/netutil"
 
 	"github.com/tendermint/tendermint/crypto"
 	"github.com/tendermint/tendermint/p2p/conn"
 )
 
@@ -120,15 +121,22 @@ func MultiplexTransportFilterTimeout(
 // net.DefaultResolver.
 func MultiplexTransportResolver(resolver IPResolver) MultiplexTransportOption {
 	return func(mt *MultiplexTransport) { mt.resolver = resolver }
 }
 
+// MultiplexTransportMaxIncomingConnections sets the maximum number of
+// simultaneous connections (incoming). Default: 0 (unlimited)
+func MultiplexTransportMaxIncomingConnections(n int) MultiplexTransportOption {
+	return func(mt *MultiplexTransport) { mt.maxIncomingConnections = n }
+}
+
 // MultiplexTransport accepts and dials tcp connections and upgrades them to
 // multiplexed peers.
 type MultiplexTransport struct {
-	netAddr  NetAddress
-	listener net.Listener
+	netAddr                NetAddress
+	listener               net.Listener
+	maxIncomingConnections int // see MaxIncomingConnections
 
 	acceptc chan accept
 	closec  chan struct{}
 
 	// Lookup table for duplicate ip and id checks.
@@ -238,10 +246,14 @@ func (mt *MultiplexTransport) Listen(addr NetAddress) error {
 	ln, err := net.Listen("tcp", addr.DialString())
 	if err != nil {
 		return err
 	}
 
+	if mt.maxIncomingConnections > 0 {
+		ln = netutil.LimitListener(ln, mt.maxIncomingConnections)
+	}
+
 	mt.netAddr = addr
 	mt.listener = ln
 
 	go mt.acceptPeers()
 
diff --git a/p2p/transport_test.go b/p2p/transport_test.go
index dd0457fb5..2fc69ce05 100644
--- a/p2p/transport_test.go
+++ b/p2p/transport_test.go
@@ -3,10 +3,11 @@ package p2p
 import (
 	"fmt"
 	"math/rand"
 	"net"
 	"reflect"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/tendermint/tendermint/crypto/ed25519"
 	"github.com/tendermint/tendermint/p2p/conn"
@@ -132,10 +133,54 @@ func TestTransportMultiplexConnFilterTimeout(t *testing.T) {
 	if _, ok := err.(ErrFilterTimeout); !ok {
 		t.Errorf("expected ErrFilterTimeout")
 	}
 }
 
+func TestTransportMultiplexMaxIncomingConnections(t *testing.T) {
+	mt := newMultiplexTransport(
+		emptyNodeInfo(),
+		NodeKey{
+			PrivKey: ed25519.GenPrivKey(),
+		},
+	)
+	id := mt.nodeKey.ID()
+
+	MultiplexTransportMaxIncomingConnections(0)(mt)
+
+	addr, err := NewNetAddressString(IDAddressString(id, "127.0.0.1:0"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if err := mt.Listen(*addr); err != nil {
+		t.Fatal(err)
+	}
+
+	errc := make(chan error)
+
+	go func() {
+		addr := NewNetAddress(id, mt.listener.Addr())
+
+		_, err := addr.Dial()
+		if err != nil {
+			errc <- err
+			return
+		}
+
+		close(errc)
+	}()
+
+	if err := <-errc; err != nil {
+		t.Errorf("connection failed: %v", err)
+	}
+
+	_, err = mt.Accept(peerConfig{})
+	if err == nil || !strings.Contains(err.Error(), "connection reset by peer") {
+		t.Errorf("expected connection reset by peer error, got %v", err)
+	}
+}
+
 func TestTransportMultiplexAcceptMultiple(t *testing.T) {
 	mt := testSetupMultiplexTransport(t)
 	laddr := NewNetAddress(mt.nodeKey.ID(), mt.listener.Addr())
 
 	var (

Vulnerable code: 	netAddr  NetAddress
	listener net.Listener
Fixed code: 
	// Limit the number of incoming connections.
	max := config.P2P.MaxNumInboundPeers + len(splitAndTrimEmpty(config.P2P.UnconditionalPeerIDs, ",", " "))
	p2p.MultiplexTransportMaxIncomingConnections(max)(transport)

	"golang.org/x/net/netutil"
// MultiplexTransportMaxIncomingConnections sets the maximum number of
// simultaneous connections (incoming). Default: 0 (unlimited)
func MultiplexTransportMaxIncomingConnections(n int) MultiplexTransportOption {
	return func(mt *MultiplexTransport) { mt.maxIncomingConnections = n }
}

	netAddr                NetAddress
	listener               net.Listener
	maxIncomingConnections int // see MaxIncomingConnections
	if mt.maxIncomingConnections > 0 {
		ln = netutil.LimitListener(ln, mt.maxIncomingConnections)
	}

	"strings"
func TestTransportMultiplexMaxIncomingConnections(t *testing.T) {
	mt := newMultiplexTransport(
		emptyNodeInfo(),
		NodeKey{
			PrivKey: ed25519.GenPrivKey(),
		},
	)
	id := mt.nodeKey.ID()

	MultiplexTransportMaxIncomingConnections(0)(mt)

	addr, err := NewNetAddressString(IDAddressString(id, "127.0.0.1:0"))
	if err != nil {
		t.Fatal(err)
	}

	if err := mt.Listen(*addr); err != nil {
		t.Fatal(err)
	}

	errc := make(chan error)

	go func() {
		addr := NewNetAddress(id, mt.listener.Addr())

		_, err := addr.Dial()
		if err != nil {
			errc <- err
			return
		}

		close(errc)
	}()

	if err := <-errc; err != nil {
		t.Errorf("connection failed: %v", err)
	}

	_, err = mt.Accept(peerConfig{})
	if err == nil || !strings.Contains(err.Error(), "connection reset by peer") {
		t.Errorf("expected connection reset by peer error, got %v", err)
	}
}