fix: TOCTOU issue in network config

beacon_node/lighthouse_network/src/discovery/mod.rs

@@ -1240,7 +1240,7 @@ mod tests {
         let spec = Arc::new(ChainSpec::default());
         let keypair = secp256k1::Keypair::generate();
         let mut config = NetworkConfig::default();
-        config.set_listening_addr(network_utils::listen_addr::ListenAddress::unused_v4_ports());
+        config.set_listening_addr(network_utils::listen_addr::ListenAddress::zero_v4_ports());
         let config = Arc::new(config);
         let enr_key: CombinedKey = CombinedKey::from_secp256k1(&keypair);
         let next_fork_digest = [0; 4];

beacon_node/src/config.rs

@@ -17,6 +17,7 @@ use execution_layer::DEFAULT_JWT_FILE;
 use http_api::TlsConfig;
 use lighthouse_network::{Enr, Multiaddr, NetworkConfig, PeerIdSerialized, multiaddr::Protocol};
 use network_utils::listen_addr::ListenAddress;
+use network_utils::listen_addr::compute_listen_ports;
 use sensitive_url::SensitiveUrl;
 use std::collections::HashSet;
 use std::fmt::Debug;
@@ -1005,12 +1006,6 @@ pub fn parse_listening_addresses(cli_args: &ArgMatches) -> Result<ListenAddress,
             // that.
             let port = maybe_port6.unwrap_or(port);
 
-            // use zero ports if required. If not, use the given port.
-            let tcp_port = use_zero_ports
-                .then(network_utils::unused_port::unused_tcp6_port)
-                .transpose()?
-                .unwrap_or(port);
-
             if maybe_disc6_port.is_some() {
                 warn!(
                     "When listening only over IPv6, use the --discovery-port flag. The value of --discovery-port6 will be ignored."
@@ -1023,19 +1018,8 @@ pub fn parse_listening_addresses(cli_args: &ArgMatches) -> Result<ListenAddress,
                 )
             }
 
-            // use zero ports if required. If not, use the specific udp port. If none given, use
-            // the tcp port.
-            let disc_port = use_zero_ports
-                .then(network_utils::unused_port::unused_udp6_port)
-                .transpose()?
-                .or(maybe_disc_port)
-                .unwrap_or(tcp_port);
-
-            let quic_port = use_zero_ports
-                .then(network_utils::unused_port::unused_udp6_port)
-                .transpose()?
-                .or(maybe_quic_port)
-                .unwrap_or(if tcp_port == 0 { 0 } else { tcp_port + 1 });
+            let (tcp_port, disc_port, quic_port) =
+                compute_listen_ports(use_zero_ports, port, maybe_disc_port, maybe_quic_port);
 
             ListenAddress::V6(network_utils::listen_addr::ListenAddr {
                 addr: ipv6,
@@ -1046,26 +1030,8 @@ pub fn parse_listening_addresses(cli_args: &ArgMatches) -> Result<ListenAddress,
         }
         (Some(ipv4), None) => {
             // A single ipv4 address was provided. Set the ports
-
-            // use zero ports if required. If not, use the given port.
-            let tcp_port = use_zero_ports
-                .then(network_utils::unused_port::unused_tcp4_port)
-                .transpose()?
-                .unwrap_or(port);
-            // use zero ports if required. If not, use the specific discovery port. If none given, use
-            // the tcp port.
-            let disc_port = use_zero_ports
-                .then(network_utils::unused_port::unused_udp4_port)
-                .transpose()?
-                .or(maybe_disc_port)
-                .unwrap_or(tcp_port);
-            // use zero ports if required. If not, use the specific quic port. If none given, use
-            // the tcp port + 1.
-            let quic_port = use_zero_ports
-                .then(network_utils::unused_port::unused_udp4_port)
-                .transpose()?
-                .or(maybe_quic_port)
-                .unwrap_or(if tcp_port == 0 { 0 } else { tcp_port + 1 });
+            let (tcp_port, disc_port, quic_port) =
+                compute_listen_ports(use_zero_ports, port, maybe_disc_port, maybe_quic_port);
 
             ListenAddress::V4(network_utils::listen_addr::ListenAddr {
                 addr: ipv4,
@@ -1078,44 +1044,13 @@ pub fn parse_listening_addresses(cli_args: &ArgMatches) -> Result<ListenAddress,
             // If --port6 is not set, we use --port
             let port6 = maybe_port6.unwrap_or(port);
 
-            let ipv4_tcp_port = use_zero_ports
-                .then(network_utils::unused_port::unused_tcp4_port)
-                .transpose()?
-                .unwrap_or(port);
-            let ipv4_disc_port = use_zero_ports
-                .then(network_utils::unused_port::unused_udp4_port)
-                .transpose()?
-                .or(maybe_disc_port)
-                .unwrap_or(ipv4_tcp_port);
-            let ipv4_quic_port = use_zero_ports
-                .then(network_utils::unused_port::unused_udp4_port)
-                .transpose()?
-                .or(maybe_quic_port)
-                .unwrap_or(if ipv4_tcp_port == 0 {
-                    0
-                } else {
-                    ipv4_tcp_port + 1
-                });
-
-            // Defaults to 9000 when required
-            let ipv6_tcp_port = use_zero_ports
-                .then(network_utils::unused_port::unused_tcp6_port)
-                .transpose()?
-                .unwrap_or(port6);
-            let ipv6_disc_port = use_zero_ports
-                .then(network_utils::unused_port::unused_udp6_port)
-                .transpose()?
-                .or(maybe_disc6_port)
-                .unwrap_or(ipv6_tcp_port);
-            let ipv6_quic_port = use_zero_ports
-                .then(network_utils::unused_port::unused_udp6_port)
-                .transpose()?
-                .or(maybe_quic6_port)
-                .unwrap_or(if ipv6_tcp_port == 0 {
-                    0
-                } else {
-                    ipv6_tcp_port + 1
-                });
+            // Compute IPv4 ports
+            let (ipv4_tcp_port, ipv4_disc_port, ipv4_quic_port) =
+                compute_listen_ports(use_zero_ports, port, maybe_disc_port, maybe_quic_port);
+
+            // Compute IPv6 ports
+            let (ipv6_tcp_port, ipv6_disc_port, ipv6_quic_port) =
+                compute_listen_ports(use_zero_ports, port6, maybe_disc6_port, maybe_quic6_port);
 
             ListenAddress::DualStack(
                 network_utils::listen_addr::ListenAddr {

common/network_utils/src/listen_addr.rs

@@ -84,21 +84,101 @@ impl ListenAddress {
             .chain(v6_tcp_multiaddr)
     }
 
-    pub fn unused_v4_ports() -> Self {
+    // Used for testing
+    pub fn zero_v4_ports() -> Self {
         ListenAddress::V4(ListenAddr {
             addr: Ipv4Addr::UNSPECIFIED,
-            disc_port: crate::unused_port::unused_udp4_port().unwrap(),
-            quic_port: crate::unused_port::unused_udp4_port().unwrap(),
-            tcp_port: crate::unused_port::unused_tcp4_port().unwrap(),
+            disc_port: 0,
+            quic_port: 0,
+            tcp_port: 0,
         })
     }
 
-    pub fn unused_v6_ports() -> Self {
+    pub fn zero_v6_ports() -> Self {
         ListenAddress::V6(ListenAddr {
             addr: Ipv6Addr::UNSPECIFIED,
-            disc_port: crate::unused_port::unused_udp6_port().unwrap(),
-            quic_port: crate::unused_port::unused_udp6_port().unwrap(),
-            tcp_port: crate::unused_port::unused_tcp6_port().unwrap(),
+            disc_port: 0,
+            quic_port: 0,
+            tcp_port: 0,
         })
     }
 }
+
+/// Compute all beacon listening ports (TCP, discovery UDP, QUIC UDP) at once.
+/// Returns a tuple of (tcp_port, disc_port, quic_port).
+///
+/// When `use_zero_ports` is true, all ports are set to 0 (OS assigns ephemeral ports).
+/// Otherwise:
+/// - TCP port uses the provided `tcp_port`
+/// - Discovery port defaults to TCP port (TCP and UDP can share the same port number)
+/// - QUIC port defaults to TCP port + 1 (to avoid conflict with discovery UDP)
+pub fn compute_listen_ports(
+    use_zero_ports: bool,
+    tcp_port: u16,
+    maybe_disc_port: Option<u16>,
+    maybe_quic_port: Option<u16>,
+) -> (u16, u16, u16) {
+    if use_zero_ports {
+        return (0, 0, 0);
+    }
+
+    let disc_port = maybe_disc_port.unwrap_or(tcp_port); // udp / tcp can listen to same port
+
+    // Handle QUIC port with overflow safety
+    let quic_port = maybe_quic_port.unwrap_or_else(|| {
+        if tcp_port == 0 || tcp_port == u16::MAX {
+            // If tcp_port is 0 or MAX, set quic_port to 0
+            0
+        } else {
+            tcp_port.wrapping_add(1)
+        }
+    });
+
+    (tcp_port, disc_port, quic_port)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_compute_listen_ports_with_zero_ports_flag() {
+        // When use_zero_ports is true, all ports should be 0 regardless of input
+        assert_eq!(compute_listen_ports(true, 9000, None, None), (0, 0, 0));
+    }
+
+    #[test]
+    fn test_compute_listen_ports_default_behavior() {
+        // Default behavior: disc_port = tcp_port, quic_port = tcp_port + 1
+        let (tcp, disc, quic) = compute_listen_ports(false, 9000, None, None);
+        assert_eq!(tcp, 9000);
+        assert_eq!(disc, 9000); // Discovery defaults to TCP port (UDP and TCP can listen to same port)
+        assert_eq!(quic, 9001); // QUIC defaults to TCP port + 1
+    }
+
+    #[test]
+    fn test_compute_listen_ports_with_explicit_ports() {
+        // Explicit discovery port should be used
+        let (tcp, disc, quic) = compute_listen_ports(false, 9000, Some(8000),   Some(7000));
+        assert_eq!(tcp, 9000);
+        assert_eq!(disc, 8000); // Explicit discovery port
+        assert_eq!(quic, 7000); // Explicit QUIC port
+    }
+
+    #[test]
+    fn test_compute_listen_ports_with_zero_tcp_port() {
+        // Edge case: tcp_port = 0 without use_zero_ports flag
+        // QUIC and discovery ports should also be 0
+        assert_eq!(compute_listen_ports(false, 0, None, None), (0, 0, 0));
+    }
+
+    #[test]
+    fn test_compute_listen_ports_max_port_overflow() {
+        // Edge case: tcp_port = u16::MAX (65535)
+        // QUIC should be 0 to avoid overflow panic
+        let (tcp, disc, quic) = compute_listen_ports(false, u16::MAX, None, None);
+        assert_eq!(tcp, u16::MAX);
+        assert_eq!(disc, u16::MAX);
+        assert_eq!(quic, 0); // u16::MAX would overflow, so we use 0
+    }
+}

common/network_utils/src/unused_port.rs

@@ -26,21 +26,6 @@ pub fn unused_tcp4_port() -> Result<u16, String> {
     zero_port(Transport::Tcp, IpVersion::Ipv4)
 }
 
-/// A convenience wrapper over [`zero_port`].
-pub fn unused_udp4_port() -> Result<u16, String> {
-    zero_port(Transport::Udp, IpVersion::Ipv4)
-}
-
-/// A convenience wrapper over [`zero_port`].
-pub fn unused_tcp6_port() -> Result<u16, String> {
-    zero_port(Transport::Tcp, IpVersion::Ipv6)
-}
-
-/// A convenience wrapper over [`zero_port`].
-pub fn unused_udp6_port() -> Result<u16, String> {
-    zero_port(Transport::Udp, IpVersion::Ipv6)
-}
-
 /// A bit of hack to find an unused port.
 ///
 /// Does not guarantee that the given port is unused after the function exits, just that it was
@@ -97,3 +82,31 @@ fn find_unused_port(transport: Transport, socket_addr: SocketAddr) -> Result<u16
 
     Ok(local_addr.port())
 }
+
+/// Bind a TCPv4 listener on localhost with an ephemeral port (port 0) and return it.
+/// Safe against TOCTOU: the socket remains open and reserved by the OS.
+pub fn bind_tcp4_any() -> Result<TcpListener, String> {
+    let addr = std::net::SocketAddr::new(std::net::Ipv4Addr::LOCALHOST.into(), 0);
+    TcpListener::bind(addr).map_err(|e| format!("Failed to bind TCPv4 listener: {:?}", e))
+}
+
+/// Bind a TCPv6 listener on localhost with an ephemeral port (port 0) and return it.
+/// Safe against TOCTOU: the socket remains open and reserved by the OS.
+pub fn bind_tcp6_any() -> Result<TcpListener, String> {
+    let addr = std::net::SocketAddr::new(std::net::Ipv6Addr::LOCALHOST.into(), 0);
+    TcpListener::bind(addr).map_err(|e| format!("Failed to bind TCPv6 listener: {:?}", e))
+}
+
+/// Bind a UDPv4 socket on localhost with an ephemeral port (port 0) and return it.
+/// Safe against TOCTOU: the socket remains open and reserved by the OS.
+pub fn bind_udp4_any() -> Result<UdpSocket, String> {
+    let addr = std::net::SocketAddr::new(std::net::Ipv4Addr::LOCALHOST.into(), 0);
+    UdpSocket::bind(addr).map_err(|e| format!("Failed to bind UDPv4 socket: {:?}", e))
+}
+
+/// Bind a UDPv6 socket on localhost with an ephemeral port (port 0) and return it.
+/// Safe against TOCTOU: the socket remains open and reserved by the OS.
+pub fn bind_udp6_any() -> Result<UdpSocket, String> {
+    let addr = std::net::SocketAddr::new(std::net::Ipv6Addr::LOCALHOST.into(), 0);
+    UdpSocket::bind(addr).map_err(|e| format!("Failed to bind UDPv6 socket: {:?}", e))
+}