fix(system): reject partially-numeric .ipc.open ports - #440
Conversation
.ipc.open parsed the port field with atoi(), which stops at the first
non-digit — so "127.0.0.1:1abc" was accepted as port 1 and a connection was
attempted instead of rejecting the malformed field. ("abc" and "70000"
already errored: atoi("abc")==0 fails the <=0 check and 70000 fails >65535, so
only the numeric-prefix case slipped through.)
Parse with strtol and require the whole field to be digits (no trailing
characters), keeping the 1..65535 range check plus ERANGE. Malformed ports now
return a domain error at parse time, before any socket work.
Adds "1abc" / "19876x" coverage to system/system_branch_cov2.rfl.
singaraiona
left a comment
There was a problem hiding this comment.
Reviewed: strict strtol parse is correct — empty/partial fields hit the *endp != '\0' check, ERANGE and range are both covered, and the buffer is NUL-terminated before parsing. Only nit (pre-existing, not a regression): strtol still accepts leading whitespace and an explicit + sign ("localhost: 80", "+80"), so the field isn't literally digits-only as the comment says. Not blocking. CI green — merging.
|
Follow-up worth filing: the strict parse landed only in |
How it looks from the user's side
A user opens an IPC connection but the port field has a typo, or trailing garbage —
1abcinstead of1234:Before —
atoi("1abc")stops at the first non-digit and returns1, so the client silently targets the wrong port. The user gets a confusing error naming a port they never typed:After — the malformed field is rejected up front, before any socket work, with a message that names the actual input:
(
"abc"and"70000"already errored —atoi("abc") == 0fails the<= 0check and70000fails> 65535— so only the numeric-prefix case slipped through.)Fix
.ipc.openparsed the port field withatoi(), which stops at the first non-digit. Parse it withstrtolinstead and require the whole field to be digits (reject any trailing characters), keeping the1..65535range check plusERANGE. A malformed port is now adomainerror at parse time, before any connection attempt.Tests
Adds
"1abc"/"19876x"cases totest/rfl/system/system_branch_cov2.rfl, alongside the existingabc/70000/-1/0port-validation checks.Full ASan+UBSan suite green:
3712 of 3713 passed (1 skipped, 0 failed).