Fix TOCTOU vulnerability in unused_port module

.github/mergify.yml

@@ -105,6 +105,10 @@ queue_rules:
 
 
         {{ body | get_section("## Proposed Changes", "") }}
+
+      {% for commit in commits | unique(attribute='email_author') %}
+      Co-Authored-By: {{ commit.author }} <{{ commit.email_author }}>
+      {% endfor %}
     queue_conditions:
       - "#approved-reviews-by >= 1"
       - "check-success=license/cla"

beacon_node/lighthouse_network/src/listen_addr.rs

@@ -88,19 +88,37 @@ impl ListenAddress {
     pub fn unused_v4_ports() -> Self {
         ListenAddress::V4(ListenAddr {
             addr: Ipv4Addr::UNSPECIFIED,
-            disc_port: unused_port::unused_udp4_port().unwrap(),
-            quic_port: unused_port::unused_udp4_port().unwrap(),
-            tcp_port: unused_port::unused_tcp4_port().unwrap(),
+            disc_port: 0,
+            quic_port: 0,
+            tcp_port: 0,
         })
     }
 
     #[cfg(test)]
     pub fn unused_v6_ports() -> Self {
         ListenAddress::V6(ListenAddr {
             addr: Ipv6Addr::UNSPECIFIED,
-            disc_port: unused_port::unused_udp6_port().unwrap(),
-            quic_port: unused_port::unused_udp6_port().unwrap(),
-            tcp_port: unused_port::unused_tcp6_port().unwrap(),
+            disc_port: 0,
+            quic_port: 0,
+            tcp_port: 0,
         })
     }
 }
+
+/// Compute the UDP discovery port given flags and TCP port.
+pub fn compute_discovery_port(use_zero_ports: bool, tcp_port: u16, maybe_disc_port: Option<u16>) -> u16 {
+    if use_zero_ports {
+        0
+    } else {
+        maybe_disc_port.unwrap_or(tcp_port)
+    }
+}
+
+/// Compute the UDP QUIC port given flags and TCP port.
+pub fn compute_quic_port(use_zero_ports: bool, tcp_port: u16, maybe_quic_port: Option<u16>) -> u16 {
+    if use_zero_ports {
+        0
+    } else {
+        maybe_quic_port.unwrap_or(if tcp_port == 0 { 0 } else { tcp_port + 1 })
+    }
+}

beacon_node/src/config.rs

@@ -1020,10 +1020,7 @@ pub fn parse_listening_addresses(cli_args: &ArgMatches) -> Result<ListenAddress,
             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(unused_port::unused_tcp6_port)
-                .transpose()?
-                .unwrap_or(port);
+            let tcp_port = if use_zero_ports { 0 } else { 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.")
@@ -1035,17 +1032,17 @@ 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(unused_port::unused_udp6_port)
-                .transpose()?
-                .or(maybe_disc_port)
-                .unwrap_or(tcp_port);
-
-            let quic_port = use_zero_ports
-                .then(unused_port::unused_udp6_port)
-                .transpose()?
-                .or(maybe_quic_port)
-                .unwrap_or(if tcp_port == 0 { 0 } else { tcp_port + 1 });
+            let disc_port = lighthouse_network::listen_addr::compute_discovery_port(
+                use_zero_ports,
+                tcp_port,
+                maybe_disc_port,
+            );
+
+            let quic_port = lighthouse_network::listen_addr::compute_quic_port(
+                use_zero_ports,
+                tcp_port,
+                maybe_quic_port,
+            );
 
             ListenAddress::V6(lighthouse_network::ListenAddr {
                 addr: ipv6,
@@ -1058,24 +1055,21 @@ pub fn parse_listening_addresses(cli_args: &ArgMatches) -> Result<ListenAddress,
             // 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(unused_port::unused_tcp4_port)
-                .transpose()?
-                .unwrap_or(port);
+            let tcp_port = if use_zero_ports { 0 } else { 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(unused_port::unused_udp4_port)
-                .transpose()?
-                .or(maybe_disc_port)
-                .unwrap_or(tcp_port);
+            let disc_port = lighthouse_network::listen_addr::compute_discovery_port(
+                use_zero_ports,
+                tcp_port,
+                maybe_disc_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(unused_port::unused_udp4_port)
-                .transpose()?
-                .or(maybe_quic_port)
-                .unwrap_or(if tcp_port == 0 { 0 } else { tcp_port + 1 });
+            let quic_port = lighthouse_network::listen_addr::compute_quic_port(
+                use_zero_ports,
+                tcp_port,
+                maybe_quic_port,
+            );
 
             ListenAddress::V4(lighthouse_network::ListenAddr {
                 addr: ipv4,
@@ -1088,44 +1082,30 @@ 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(unused_port::unused_tcp4_port)
-                .transpose()?
-                .unwrap_or(port);
-            let ipv4_disc_port = use_zero_ports
-                .then(unused_port::unused_udp4_port)
-                .transpose()?
-                .or(maybe_disc_port)
-                .unwrap_or(ipv4_tcp_port);
-            let ipv4_quic_port = use_zero_ports
-                .then(unused_port::unused_udp4_port)
-                .transpose()?
-                .or(maybe_quic_port)
-                .unwrap_or(if ipv4_tcp_port == 0 {
-                    0
-                } else {
-                    ipv4_tcp_port + 1
-                });
+            let ipv4_tcp_port = if use_zero_ports { 0 } else { port };
+            let ipv4_disc_port = lighthouse_network::listen_addr::compute_discovery_port(
+                use_zero_ports,
+                ipv4_tcp_port,
+                maybe_disc_port,
+            );
+            let ipv4_quic_port = lighthouse_network::listen_addr::compute_quic_port(
+                use_zero_ports,
+                ipv4_tcp_port,
+                maybe_quic_port,
+            );
 
             // Defaults to 9000 when required
-            let ipv6_tcp_port = use_zero_ports
-                .then(unused_port::unused_tcp6_port)
-                .transpose()?
-                .unwrap_or(port6);
-            let ipv6_disc_port = use_zero_ports
-                .then(unused_port::unused_udp6_port)
-                .transpose()?
-                .or(maybe_disc6_port)
-                .unwrap_or(ipv6_tcp_port);
-            let ipv6_quic_port = use_zero_ports
-                .then(unused_port::unused_udp6_port)
-                .transpose()?
-                .or(maybe_quic6_port)
-                .unwrap_or(if ipv6_tcp_port == 0 {
-                    0
-                } else {
-                    ipv6_tcp_port + 1
-                });
+            let ipv6_tcp_port = if use_zero_ports { 0 } else { port6 };
+            let ipv6_disc_port = lighthouse_network::listen_addr::compute_discovery_port(
+                use_zero_ports,
+                ipv6_tcp_port,
+                maybe_disc6_port,
+            );
+            let ipv6_quic_port = lighthouse_network::listen_addr::compute_quic_port(
+                use_zero_ports,
+                ipv6_tcp_port,
+                maybe_quic6_port,
+            );
 
             ListenAddress::DualStack(
                 lighthouse_network::ListenAddr {

common/unused_port/src/lib.rs

@@ -22,25 +22,57 @@ static FOUND_PORTS_CACHE: LazyLock<Mutex<LRUTimeCache<u16>>> =
     LazyLock::new(|| Mutex::new(LRUTimeCache::new(CACHED_PORTS_TTL)));
 
 /// A convenience wrapper over [`zero_port`].
+#[deprecated(note = "Use bind_tcp4_any() which returns a bound socket to avoid TOCTOU")]
 pub fn unused_tcp4_port() -> Result<u16, String> {
     zero_port(Transport::Tcp, IpVersion::Ipv4)
 }
 
 /// A convenience wrapper over [`zero_port`].
+#[deprecated(note = "Use bind_udp4_any() which returns a bound socket to avoid TOCTOU")]
 pub fn unused_udp4_port() -> Result<u16, String> {
     zero_port(Transport::Udp, IpVersion::Ipv4)
 }
 
 /// A convenience wrapper over [`zero_port`].
+#[deprecated(note = "Use bind_tcp6_any() which returns a bound socket to avoid TOCTOU")]
 pub fn unused_tcp6_port() -> Result<u16, String> {
     zero_port(Transport::Tcp, IpVersion::Ipv6)
 }
 
 /// A convenience wrapper over [`zero_port`].
+#[deprecated(note = "Use bind_udp6_any() which returns a bound socket to avoid TOCTOU")]
 pub fn unused_udp6_port() -> Result<u16, String> {
     zero_port(Transport::Udp, IpVersion::Ipv6)
 }
 
+/// 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))
+}
+
 /// 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
@@ -51,6 +83,7 @@ pub fn unused_udp6_port() -> Result<u16, String> {
 /// It is possible that users are unable to bind to the ports returned by this function as the OS
 /// has a buffer period where it doesn't allow binding to the same port even after the socket is
 /// closed. We might have to use SO_REUSEADDR socket option from `std::net2` crate in that case.
+#[deprecated(note = "Use bind_*_any() functions that return a bound socket to avoid TOCTOU")]
 pub fn zero_port(transport: Transport, ipv: IpVersion) -> Result<u16, String> {
     let localhost = match ipv {
         IpVersion::Ipv4 => std::net::Ipv4Addr::LOCALHOST.into(),

lighthouse/tests/beacon_node.rs

@@ -21,7 +21,6 @@ use std::time::Duration;
 use tempfile::TempDir;
 use types::non_zero_usize::new_non_zero_usize;
 use types::{Address, Checkpoint, Epoch, Hash256, MainnetEthSpec};
-use unused_port::{unused_tcp4_port, unused_tcp6_port, unused_udp4_port, unused_udp6_port};
 
 const DEFAULT_EXECUTION_ENDPOINT: &str = "http://localhost:8551/";
 const DEFAULT_EXECUTION_JWT_SECRET_KEY: &str =
@@ -32,6 +31,12 @@ const DUMMY_ENR_TCP_PORT: u16 = 7777;
 const DUMMY_ENR_UDP_PORT: u16 = 8888;
 const DUMMY_ENR_QUIC_PORT: u16 = 9999;
 
+// Fixed test ports for config-only assertions (no actual bind occurs in these tests).
+const TEST_TCP4_PORT: u16 = 39001;
+const TEST_TCP6_PORT: u16 = 39011;
+const TEST_UDP4_PORT: u16 = 39002;
+const TEST_UDP6_PORT: u16 = 39012;
+
 const _: () =
     assert!(DUMMY_ENR_QUIC_PORT != 0 && DUMMY_ENR_TCP_PORT != 0 && DUMMY_ENR_UDP_PORT != 0);
 
@@ -1039,8 +1044,8 @@ fn network_port_flag_over_ipv4_and_ipv6() {
             );
         });
 
-    let port = unused_tcp4_port().expect("Unable to find unused port.");
-    let port6 = unused_tcp6_port().expect("Unable to find unused port.");
+    let port = TEST_TCP4_PORT;
+    let port6 = TEST_TCP6_PORT;
     CommandLineTest::new()
         .flag("listen-address", Some("127.0.0.1"))
         .flag("listen-address", Some("::1"))
@@ -1422,8 +1427,8 @@ fn enr_match_flag_over_ipv6() {
     const ADDR: &str = "::1";
     let addr = ADDR.parse::<Ipv6Addr>().unwrap();
 
-    let udp6_port = unused_udp6_port().expect("Unable to find unused port.");
-    let tcp6_port = unused_tcp6_port().expect("Unable to find unused port.");
+    let udp6_port = TEST_UDP6_PORT;
+    let tcp6_port = TEST_TCP6_PORT;
 
     CommandLineTest::new()
         .flag("enr-match", None)
@@ -1452,13 +1457,13 @@ fn enr_match_flag_over_ipv6() {
 fn enr_match_flag_over_ipv4_and_ipv6() {
     const IPV6_ADDR: &str = "::1";
 
-    let udp6_port = unused_udp6_port().expect("Unable to find unused port.");
-    let tcp6_port = unused_tcp6_port().expect("Unable to find unused port.");
+    let udp6_port = TEST_UDP6_PORT;
+    let tcp6_port = TEST_TCP6_PORT;
     let ipv6_addr = IPV6_ADDR.parse::<Ipv6Addr>().unwrap();
 
     const IPV4_ADDR: &str = "127.0.0.1";
-    let udp4_port = unused_udp4_port().expect("Unable to find unused port.");
-    let tcp4_port = unused_tcp4_port().expect("Unable to find unused port.");
+    let udp4_port = TEST_UDP4_PORT;
+    let tcp4_port = TEST_TCP4_PORT;
     let ipv4_addr = IPV4_ADDR.parse::<Ipv4Addr>().unwrap();
 
     CommandLineTest::new()

lighthouse/tests/boot_node.rs

@@ -12,10 +12,12 @@ use std::path::{Path, PathBuf};
 use std::process::Command;
 use std::str::FromStr;
 use tempfile::TempDir;
-use unused_port::unused_udp4_port;
 
 const IP_ADDRESS: &str = "192.168.2.108";
 
+/ Fixed test port for config-only assertions (no actual bind occurs in these tests).
+const TEST_UDP4_PORT: u16 = 39102;
+
 /// Returns the `lighthouse boot_node` command.
 fn base_cmd() -> Command {
     let lighthouse_bin = env!("CARGO_BIN_EXE_lighthouse");
@@ -62,7 +64,7 @@ fn enr_address_arg() {
 
 #[test]
 fn port_flag() {
-    let port = unused_udp4_port().unwrap();
+    let port = TEST_UDP4_PORT;
     CommandLineTest::new()
         .flag("port", Some(port.to_string().as_str()))
         .run_with_ip()
@@ -134,7 +136,7 @@ fn boot_nodes_flag() {
 
 #[test]
 fn enr_port_flag() {
-    let port = unused_udp4_port().unwrap();
+    let port = TEST_UDP4_PORT;
     CommandLineTest::new()
         .flag("enr-port", Some(port.to_string().as_str()))
         .run_with_ip()