netcode_generate_connect_token: bound num_server_addresses at runtime (#173)

[rowan-claude]

Fixes #173.

The two bounds on num_server_addresses were netcode_assert only. Release builds define NDEBUG, those compile to ((void)0), and the parse loops write into parsed_public_server_addresses[NETCODE_MAX_SERVERS_PER_CONNECT] — a 32-element stack array — with no runtime bound.

This completes an existing decision, it does not introduce a policy

netcode.c:4101-4103 already states the rule outright:

an out of range value here must not get through in release builds where asserts compile out

…and the same assert+check pairing is on netcode_client_send_packet (3387), netcode_server_start (4104), netcode_server_send_packet (4998, 5010), and every client_index accessor. netcode_generate_connect_token is the one public entry point that was skipped. The tell is internal to the function: the same loop body already returns NETCODE_ERROR for a malformed address string.

Severity

Caller-controlled, not reachable from the wire — the count comes from whatever mints tokens. A precondition violation, not a remote vulnerability, which is why it was not filed as an advisory. The wire-facing path (netcode_read_packet, netcode_read_connect_token_private) validates before every write and is unaffected.

It cannot break a valid caller: anyone passing an out-of-range count was already in undefined behaviour, and this only turns that into a documented error return.

The test had to defeat the asserts to reach the release path

The asserts fire first by design, so test_generate_connect_token_out_of_range installs an assert handler that returns — the behaviour documented at netcode.h:362-366 for exactly this situation. It then:

• checks all three out-of-range values return NETCODE_ERROR
• checks the asserts still fired, so it cannot quietly pass by proving they were removed
• checks an in-range call still returns NETCODE_OK, so it cannot pass by rejecting everything
• restores &netcode_default_assert_handler rather than NULL — netcode_assert calls the pointer with no null guard, so NULL would turn the next failing assert anywhere in the suite into a crash

Verified to fail without the fix (suite exits 133) and pass with it.

Deliberately not changed

netcode_parse_address("[::1") still returns OK and yields ::1. I had this queued as a tightening and dropped it: that is permissive-on-receive and it is intended. #174 should close as by-design.

Likewise nothing was added to any write path — asserts there are the contract, and the caller owns not invoking UB.

🤖 Generated with Claude Code