From 681d32737004279287deeed4a05ff92abe629c90 Mon Sep 17 00:00:00 2001 From: felipe <130432649+felipe-op@users.noreply.github.com> Date: Mon, 18 Mar 2024 18:56:15 -0700 Subject: [PATCH] feat(op-node): p2p rpc input validation (#9897) --- op-node/p2p/host_test.go | 11 ++++++++ op-node/p2p/rpc_server.go | 53 +++++++++++++++++++++++++++++++++------ 2 files changed, 56 insertions(+), 8 deletions(-) diff --git a/op-node/p2p/host_test.go b/op-node/p2p/host_test.go index 5fabfe48e950..a71372a08967 100644 --- a/op-node/p2p/host_test.go +++ b/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) diff --git a/op-node/p2p/rpc_server.go b/op-node/p2p/rpc_server.go index 1b908dd45e61..1a9c7fcb8f2b 100644 --- a/op-node/p2p/rpc_server.go +++ b/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 {