lighthouse icon indicating copy to clipboard operation
lighthouse copied to clipboard
verified publisher icon sigp

fix: TOCTOU issue in network config

Open 0xmrree opened this issue 3 weeks ago • 1 comments

Fixes TOCTOU issue in beacon node by replacing unsafe port-finding with OS-assigned ports (port 0), ensuring atomic port allocation without race conditions. (Basicly fixing a theoritcal race condition that would prevent the node to start up) Production beacon client is fully fixed with new compute_listen_ports() helper; integration tests retain deprecated approach temporarily pending maintainer input.

Issue Addressed

Which issue # does this PR address? - https://github.com/sigp/lighthouse/issues/8490

Proposed Changes

Changes Introduced

  • Fixes TOCTOU vulnerability in beacon node network port allocation by using OS-assigned ports (port 0)
  • Adds compute_listen_ports() helper function in listen_addr.rs to centralize port computation logic for TCP, discovery UDP, and QUIC UDP ports
  • Implements overflow-safe port handling
  • Adds unit tests covering zero-ports flag, explicit port overrides, and overflow scenarios
  • Updates beacon_node/src/config.rs to use new helper for IPv4, IPv6, and dual-stack configurations
  • Adds integration tests in lighthouse/tests/beacon_node.rs to verify --zero-ports flag behavior
  • Provides safe bind_*_any() APIs for future migration of test code (currently unused)
  • Retains one of the deprecated functions for execution_engine_integration tests pending maintainer guidance - the rest of the old methods were removed

What's Fixed / Additional Info

  • Production beacon node: fully protected against port allocation races
  • Integration tests: still use deprecated approach (to be addressed separately)

Next steps

I think this PR should be merged as is, but we need to address the CL/EL integration test issues. Here I need help from maintainers. Basically, the EL/CL integration tests are still using the old approach that has the TOCTOU issue because after we start the EL process, we need to pass the ports chosen by the EL downstream, which is not easily possible without parsing stdout of the child process—a fragile approach I'm not sure you want to take. That being said, outside of trying to pass a file descriptor into the CLI of EL, this is the only approach I can think of.

Let me know your thoughts. The relevant code is here: https://github.com/sigp/lighthouse/blob/stable/testing/execution_engine_integration/src/execution_engine.rs#L55 If you folks want me to go down the path of parsing the stdout of the EL CLI, I will make an additional PR after this is merged.

Testing

ran cargo nextest run -p lighthouse --release and passed all tests, see below

...
PASS [   0.014s] lighthouse::lighthouse_tests validator_manager::validator_exit_using_beacon_and_presign_flags
PASS [   0.019s] lighthouse::lighthouse_tests validator_manager::validator_import_defaults
PASS [   0.020s] lighthouse::lighthouse_tests validator_manager::validator_import_misc_flags
PASS [   0.015s] lighthouse::lighthouse_tests validator_manager::validator_import_missing_both_file_flags
PASS [   0.017s] lighthouse::lighthouse_tests validator_manager::validator_import_missing_token
PASS [   0.015s] lighthouse::lighthouse_tests validator_manager::validator_import_using_both_file_flags
PASS [   0.019s] lighthouse::lighthouse_tests validator_manager::validator_list_defaults
PASS [   0.020s] lighthouse::lighthouse_tests validator_manager::validator_move_count
PASS [   0.021s] lighthouse::lighthouse_tests validator_manager::validator_move_defaults
PASS [   0.019s] lighthouse::lighthouse_tests validator_manager::validator_move_misc_flags_0
PASS [   0.019s] lighthouse::lighthouse_tests validator_manager::validator_move_misc_flags_1
PASS [   0.019s] lighthouse::lighthouse_tests validator_manager::validator_move_misc_flags_2
PASS [  24.726s] lighthouse::lighthouse_tests beacon_node::validator_monitor_file_flag
PASS [  24.531s] lighthouse::lighthouse_tests beacon_node::validator_monitor_metrics_threshold_custom
PASS [  52.022s] lighthouse::lighthouse_tests beacon_node::test_builder_disable_ssz_flag
PASS [  25.477s] lighthouse::lighthouse_tests beacon_node::validator_monitor_pubkeys_flag
PASS [  25.558s] lighthouse::lighthouse_tests beacon_node::validator_monitor_metrics_threshold_default
PASS [  25.330s] lighthouse::lighthouse_tests beacon_node::wss_checkpoint_flag
PASS [  27.968s] lighthouse::lighthouse_tests beacon_node::zero_ports_flag
────────────
Summary [ 728.214s] 310 tests run: 310 passed, 0 skipped

ran cargo test -p network_utils --lib and passed all tests, see below:

...
running 13 tests
test listen_addr::tests::test_compute_listen_ports_default_behavior ... ok
test listen_addr::tests::test_compute_listen_ports_max_port_overflow ... ok
test listen_addr::tests::test_compute_listen_ports_max_port_with_explicit_quic ... ok
test listen_addr::tests::test_compute_listen_ports_quic_avoids_discovery_conflict ... ok
test listen_addr::tests::test_compute_listen_ports_tcp_and_udp_can_share_port ... ok
test listen_addr::tests::test_compute_listen_ports_with_all_explicit_ports ... ok
test listen_addr::tests::test_compute_listen_ports_with_explicit_disc_port ... ok
test listen_addr::tests::test_compute_listen_ports_with_explicit_quic_port ... ok
test listen_addr::tests::test_compute_listen_ports_with_zero_ports_flag ... ok
test listen_addr::tests::test_compute_listen_ports_with_zero_tcp_and_explicit_ports ... ok
test listen_addr::tests::test_compute_listen_ports_with_zero_tcp_port ... ok
test enr_ext::tests::test_ed25519_peer_conversion ... ok
test enr_ext::tests::test_secp256k1_peer_id_conversion ... ok

test result: ok. 13 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

ran cargo test -p lighthouse_network and passed all tests, see below:

...
tus(V2(StatusMessageV2 { fork_digest: [0, 0, 0, 0], finalized_root: 0x0000000000000000000000000000000000000000000000000000000000000000, finalized_epoch: Epoch(1), head_root: 0x0000000000000000000000000000000000000000000000000000000000000000, head_slot: Slot(1), earliest_available_slot: Slot(0) }))
test rpc_tests::test_active_requests ... ok
2025-12-08T02:40:26.947027Z DEBUG Receiver: lighthouse_network_tests::rpc_tests: Sending message 18
2025-12-08T02:40:27.949528Z DEBUG Receiver: lighthouse_network_tests::rpc_tests: Sending message 19
2025-12-08T02:40:28.952231Z DEBUG Receiver: lighthouse_network_tests::rpc_tests: Sending message 20
test rpc_tests::test_tcp_blocks_by_range_chunked_rpc_terminates_correctly ... ok

test result: ok. 14 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 20.23s

   Doc-tests lighthouse_network

running 1 test

0xmrree avatar Dec 08 '25 03:12 0xmrree

CLA assistant check
All committers have signed the CLA.

cla-assistant[bot] avatar Dec 08 '25 03:12 cla-assistant[bot]

Should be good for another review

0xmrree avatar Dec 12 '25 20:12 0xmrree