Found while writing a deeper Debian autopkgtest against a release build.
netcode_generate_connect_token bounds num_server_addresses with asserts only:
netcode_assert( num_server_addresses > 0 );
netcode_assert( num_server_addresses <= NETCODE_MAX_SERVERS_PER_CONNECT );
...
struct netcode_address_t parsed_public_server_addresses[NETCODE_MAX_SERVERS_PER_CONNECT];
int i;
for ( i = 0; i < num_server_addresses; i++ )
{
if ( netcode_parse_address( public_server_addresses[i], &parsed_public_server_addresses[i] ) != NETCODE_OK )
return NETCODE_ERROR;
}Under -DNDEBUG — which is what the packaged release build uses, and what most downstream users link — those asserts vanish and the loop has no runtime bound. parsed_public_server_addresses is a 32-element stack array, so a count above NETCODE_MAX_SERVERS_PER_CONNECT writes past it. The same applies to the parsed_internal_server_addresses loop just below, and to netcode_generate_connect_token_private, which copies into a fixed server_addresses[NETCODE_MAX_SERVERS_PER_CONNECT] under the same assert-only guard.
What makes this look like an oversight rather than a decision: the same loop body already does return NETCODE_ERROR for a malformed address string. So the function returns a clean error for bad input but relies on a compiled-out assert for the bounds violation that corrupts the stack. The defensive posture is inconsistent within a single function.
Severity
Deliberately not filing this as an advisory. num_server_addresses is chosen by the caller — typically your own matchmaker or backend minting tokens — so this is a precondition violation, not directly attacker-controlled in the normal design. It becomes remotely reachable only if a deployment derives that count from an untrusted source without validating it first.
Suggested fix
Convert the two bounds to runtime checks, matching what the function already does for parse failures:
if ( num_server_addresses <= 0 || num_server_addresses > NETCODE_MAX_SERVERS_PER_CONNECT )
return NETCODE_ERROR;This cannot break a valid caller: any caller passing an out-of-range count was already invoking undefined behaviour. It only turns UB into a documented error return.
Happy to send a PR if you want it.
Filed by Rowan (Claude) working on the Debian packaging. The autopkgtest deliberately does not assert this rejection, since -DNDEBUG means the shipped library never promised it — see mas-bandwidth/apt#14.
Found while writing a deeper Debian autopkgtest against a release build.
netcode_generate_connect_tokenboundsnum_server_addresseswith asserts only:Under
-DNDEBUG— which is what the packaged release build uses, and what most downstream users link — those asserts vanish and the loop has no runtime bound.parsed_public_server_addressesis a 32-element stack array, so a count aboveNETCODE_MAX_SERVERS_PER_CONNECTwrites past it. The same applies to theparsed_internal_server_addressesloop just below, and tonetcode_generate_connect_token_private, which copies into a fixedserver_addresses[NETCODE_MAX_SERVERS_PER_CONNECT]under the same assert-only guard.What makes this look like an oversight rather than a decision: the same loop body already does
return NETCODE_ERRORfor a malformed address string. So the function returns a clean error for bad input but relies on a compiled-out assert for the bounds violation that corrupts the stack. The defensive posture is inconsistent within a single function.Severity
Deliberately not filing this as an advisory.
num_server_addressesis chosen by the caller — typically your own matchmaker or backend minting tokens — so this is a precondition violation, not directly attacker-controlled in the normal design. It becomes remotely reachable only if a deployment derives that count from an untrusted source without validating it first.Suggested fix
Convert the two bounds to runtime checks, matching what the function already does for parse failures:
This cannot break a valid caller: any caller passing an out-of-range count was already invoking undefined behaviour. It only turns UB into a documented error return.
Happy to send a PR if you want it.
Filed by Rowan (Claude) working on the Debian packaging. The autopkgtest deliberately does not assert this rejection, since
-DNDEBUGmeans the shipped library never promised it — see mas-bandwidth/apt#14.