c-ares / c-ares / 28707169038

90%

Build Type

push

github

Committed by web-flow

Commit Message

route numeric config parsing through range-checked ares_str_parse_uint (#1158)

Consolidates the atoi->strtoul sweep into one audited path, per review.
This is the single consolidated PR; #1157 and #1162 are closed in favor
of it.

- add `ares_str_parse_uint()`, a strtoul-based helper with
errno/end/range checks, and rework `ares_parse_port()` on top of it
- route link-local interface scope, sortlist netmask (was #1157), and
`name_match` (was #1162) through it
- fold the hand-rolled numeric-service port parser in
`ares_getaddrinfo.c` into `ares_parse_port()` so there's a single
validated path
- also validate the previously-unchecked `strtoul` in the resolv.conf
options parser (`ares_sysconfig_files.c`)

`atoi()` is undefined behavior on overflow, and the numeric tokens here
are only validated as digits (up to 15 for the zone id), so an
out-of-range value like `[fe80::1]:53%4294967297` was accepted instead
of rejected. The helper rejects overflow, trailing garbage, and anything
that doesn't fit the target width.

Behavior change worth noting: a numeric `getaddrinfo()` service that the
old hand-rolled parser tolerated now returns `ARES_ESERVICE`. This
covers an empty service `""` (previously yielded port 0), and leading
whitespace/sign like `" 80"` / `"+80"`. This matches musl; glibc treats
`""` like NULL and accepts the other two. POSIX-defensible either way.

Tests: direct `ares_str_parse_uint` unit test (`StrParseUint`) covering
the boundary/overflow/garbage cases, plus a `SetServersCSV` case
rejecting an out-of-range numeric scope.

Rebased on main to resolve the `ares_getaddrinfo.c` conflict with #1186.

---------

Signed-off-by: Syed Mohammed Nayyar <jmestwa@gmail.com>

Coverage Stats

46 of 49 new or added lines in 6 files covered. (93.88%)

4 existing lines in 4 files now uncovered.

23960 of 26149 relevant lines covered (91.63%)

12081.14 hits per line