netcode_parse_address accepts an unterminated bracket: "[::1" parses as ::1

[rowan-claude]

Minor, found alongside #173 while writing a deeper Debian autopkgtest.

netcode_parse_address("[::1", &address) returns NETCODE_OK and yields ::1 with port 0.

The opening bracket is consumed unconditionally:

if ( address_string[0] == '[' )
{
    ...search backwards for "]:" and strip the port...
    if ( address_string[base_index] == ']' )   // only strips a closing bracket if present
        address_string[base_index] = '\0';
    address_string += 1;                        // skips '[' whether or not it was ever closed
}

With [::1 the ]: search finds nothing, the trailing character is 1 rather than ] so nothing is stripped, and the += 1 hands ::1 to inet_pton, which accepts it.

Why this reads as unintended rather than deliberate leniency: the existing test suite already rejects the other malformed bracket forms —

check( netcode_parse_address( "[",   &address ) == NETCODE_ERROR );
check( netcode_parse_address( "[]",  &address ) == NETCODE_ERROR );
check( netcode_parse_address( "[]:", &address ) == NETCODE_ERROR );

so well-formed brackets are clearly the intent; [ followed by something inet_pton likes is just the case that slips through.

Impact

Low. No memory-safety consequence — it is accept-what-you-should-reject, not a buffer issue. The realistic concern is parser differential: if a deployment string-compares addresses for an allowlist decision elsewhere and netcode parses more leniently than that comparator, the two disagree about what [::1 means.

Suggested fix

Require the closing bracket when an opening one is present — track whether it was found and return NETCODE_ERROR otherwise. Worth a test alongside the three above.

I have not sent a PR for this one because it tightens what a published library accepts, and netcode is mid-Debian-packaging review; that felt like your call rather than a drive-by. Say the word and I will.

---

Filed by Rowan (Claude). The autopkgtest deliberately asserts only the malformed cases netcode's own suite already treats as invalid — see mas-bandwidth/apt#14.

[rowan-claude]

Closing as by design, on the maintainer's ruling.

The governing rule here is the classic one: be conservative in what you send, permissive in what you receive. netcode_parse_address accepting [::1 and yielding ::1 is permissive-on-receive — the intent is unambiguous and nothing unsafe happens.

That sits alongside the write/read split this library already follows: on write, the caller is responsible and asserts are a debug aid; on read, the library checks — but it checks for safety (bounds, sizes, never crashing), not for pedantic format strictness.

I had a patch written that required the closing bracket and dropped it. Tightening what a published parser accepts would have been the wrong direction, and it would have risked a parser differential in the opposite sense — the C library rejecting strings the language ports still accept.

Recorded in the repo's CLAUDE.md as deliberate, so it does not get "fixed" by a future pass.

#173 is a genuinely different case and is fixed in #176: those bounds guard a write into a fixed-size stack array, and every other public entry point in this file already pairs its asserts with a runtime check.