feat(op-node): p2p rpc input validation (#9897)

op-node/p2p/host_test.go

@@ -181,6 +181,17 @@ func TestP2PFull(t *testing.T) {
 	require.Equal(t, []peer.ID{hostB.ID()}, blockedPeers)
 	require.NoError(t, p2pClientA.UnblockPeer(ctx, hostB.ID()))
 
+	require.Error(t, p2pClientA.BlockAddr(ctx, nil))
+	require.Error(t, p2pClientA.UnblockAddr(ctx, nil))
+	require.Error(t, p2pClientA.BlockSubnet(ctx, nil))
+	require.Error(t, p2pClientA.UnblockSubnet(ctx, nil))
+	require.Error(t, p2pClientA.BlockPeer(ctx, ""))
+	require.Error(t, p2pClientA.UnblockPeer(ctx, ""))
+	require.Error(t, p2pClientA.ProtectPeer(ctx, ""))
+	require.Error(t, p2pClientA.UnprotectPeer(ctx, ""))
+	require.Error(t, p2pClientA.ConnectPeer(ctx, ""))
+	require.Error(t, p2pClientA.DisconnectPeer(ctx, ""))
+
 	require.NoError(t, p2pClientA.BlockAddr(ctx, net.IP{123, 123, 123, 123}))
 	blockedIPs, err := p2pClientA.ListBlockedAddrs(ctx)
 	require.NoError(t, err)

op-node/p2p/rpc_server.go

@@ -36,6 +36,7 @@ var (
 	ErrDisabledDiscovery   = errors.New("discovery disabled")
 	ErrNoConnectionManager = errors.New("no connection manager")
 	ErrNoConnectionGater   = errors.New("no connection gater")
+	ErrInvalidRequest      = errors.New("invalid request")
 )
 
 type Node interface {
@@ -244,23 +245,31 @@ func (s *APIBackend) DiscoveryTable(_ context.Context) ([]*enode.Node, error) {
 	}
 }
 
-func (s *APIBackend) BlockPeer(_ context.Context, p peer.ID) error {
+func (s *APIBackend) BlockPeer(_ context.Context, id peer.ID) error {
 	recordDur := s.m.RecordRPCServerRequest("opp2p_blockPeer")
+	if err := id.Validate(); err != nil {
+		log.Warn("invalid peer ID", "method", "BlockPeer", "peer", id, "err", err)
+		return ErrInvalidRequest
+	}
 	defer recordDur()
 	if gater := s.node.ConnectionGater(); gater == nil {
 		return ErrNoConnectionGater
 	} else {
-		return gater.BlockPeer(p)
+		return gater.BlockPeer(id)
 	}
 }
 
-func (s *APIBackend) UnblockPeer(_ context.Context, p peer.ID) error {
+func (s *APIBackend) UnblockPeer(_ context.Context, id peer.ID) error {
 	recordDur := s.m.RecordRPCServerRequest("opp2p_unblockPeer")
+	if err := id.Validate(); err != nil {
+		log.Warn("invalid peer ID", "method", "UnblockPeer", "peer", id, "err", err)
+		return ErrInvalidRequest
+	}
 	defer recordDur()
 	if gater := s.node.ConnectionGater(); gater == nil {
 		return ErrNoConnectionGater
 	} else {
-		return gater.UnblockPeer(p)
+		return gater.UnblockPeer(id)
 	}
 }
 
@@ -278,6 +287,10 @@ func (s *APIBackend) ListBlockedPeers(_ context.Context) ([]peer.ID, error) {
 // Note: active connections to the IP address are not automatically closed.
 func (s *APIBackend) BlockAddr(_ context.Context, ip net.IP) error {
 	recordDur := s.m.RecordRPCServerRequest("opp2p_blockAddr")
+	if ip == nil {
+		log.Warn("invalid IP", "method", "BlockAddr")
+		return ErrInvalidRequest
+	}
 	defer recordDur()
 	if gater := s.node.ConnectionGater(); gater == nil {
 		return ErrNoConnectionGater
@@ -288,6 +301,10 @@ func (s *APIBackend) BlockAddr(_ context.Context, ip net.IP) error {
 
 func (s *APIBackend) UnblockAddr(_ context.Context, ip net.IP) error {
 	recordDur := s.m.RecordRPCServerRequest("opp2p_unblockAddr")
+	if ip == nil {
+		log.Warn("invalid IP", "method", "UnblockAddr")
+		return ErrInvalidRequest
+	}
 	defer recordDur()
 	if gater := s.node.ConnectionGater(); gater == nil {
 		return ErrNoConnectionGater
@@ -310,6 +327,10 @@ func (s *APIBackend) ListBlockedAddrs(_ context.Context) ([]net.IP, error) {
 // Note: active connections to the IP subnet are not automatically closed.
 func (s *APIBackend) BlockSubnet(_ context.Context, ipnet *net.IPNet) error {
 	recordDur := s.m.RecordRPCServerRequest("opp2p_blockSubnet")
+	if ipnet == nil {
+		log.Warn("invalid IPNet", "method", "BlockSubnet")
+		return ErrInvalidRequest
+	}
 	defer recordDur()
 	if gater := s.node.ConnectionGater(); gater == nil {
 		return ErrNoConnectionGater
@@ -320,6 +341,10 @@ func (s *APIBackend) BlockSubnet(_ context.Context, ipnet *net.IPNet) error {
 
 func (s *APIBackend) UnblockSubnet(_ context.Context, ipnet *net.IPNet) error {
 	recordDur := s.m.RecordRPCServerRequest("opp2p_unblockSubnet")
+	if ipnet == nil {
+		log.Warn("invalid IPNet", "method", "UnblockSubnet")
+		return ErrInvalidRequest
+	}
 	defer recordDur()
 	if gater := s.node.ConnectionGater(); gater == nil {
 		return ErrNoConnectionGater
@@ -338,24 +363,32 @@ func (s *APIBackend) ListBlockedSubnets(_ context.Context) ([]*net.IPNet, error)
 	}
 }
 
-func (s *APIBackend) ProtectPeer(_ context.Context, p peer.ID) error {
+func (s *APIBackend) ProtectPeer(_ context.Context, id peer.ID) error {
 	recordDur := s.m.RecordRPCServerRequest("opp2p_protectPeer")
+	if err := id.Validate(); err != nil {
+		log.Warn("invalid peer ID", "method", "ProtectPeer", "peer", id, "err", err)
+		return ErrInvalidRequest
+	}
 	defer recordDur()
 	if manager := s.node.ConnectionManager(); manager == nil {
 		return ErrNoConnectionManager
 	} else {
-		manager.Protect(p, "api-protected")
+		manager.Protect(id, "api-protected")
 		return nil
 	}
 }
 
-func (s *APIBackend) UnprotectPeer(_ context.Context, p peer.ID) error {
+func (s *APIBackend) UnprotectPeer(_ context.Context, id peer.ID) error {
 	recordDur := s.m.RecordRPCServerRequest("opp2p_unprotectPeer")
+	if err := id.Validate(); err != nil {
+		log.Warn("invalid peer ID", "method", "UnprotectPeer", "peer", id, "err", err)
+		return ErrInvalidRequest
+	}
 	defer recordDur()
 	if manager := s.node.ConnectionManager(); manager == nil {
 		return ErrNoConnectionManager
 	} else {
-		manager.Unprotect(p, "api-protected")
+		manager.Unprotect(id, "api-protected")
 		return nil
 	}
 }
@@ -377,6 +410,10 @@ func (s *APIBackend) ConnectPeer(ctx context.Context, addr string) error {
 
 func (s *APIBackend) DisconnectPeer(_ context.Context, id peer.ID) error {
 	recordDur := s.m.RecordRPCServerRequest("opp2p_disconnectPeer")
+	if err := id.Validate(); err != nil {
+		log.Warn("invalid peer ID", "method", "DisconnectPeer", "peer", id, "err", err)
+		return ErrInvalidRequest
+	}
 	defer recordDur()
 	err := s.node.Host().Network().ClosePeer(id)
 	if err != nil {