Re: BUG #19583: macaddr input accepts octet fields longer than 8 hex digits

On Wed, Jul 29, 2026, Daniel Gustafsson wrote:
> Please do, patches are always welcome.

Hi,

Attached is a patch bounding the two remaining unbounded %x conversions
in macaddr_in() with %2x, matching the five condensed formats in the
same function.

A few notes from testing (behavior cross-checked on glibc — PG master
and 16.13 — and two Windows C runtimes, which all agree):

* One correction to the analysis in the report: C99 does not specify
modulo behavior for an overflowing %x conversion — 7.19.6.2p10 makes
it undefined ("if the result of the conversion cannot be represented
in the object, the behavior is undefined"); mod-2^32 is just what
glibc and Apple's libc happen to do. That makes the status quo a bit
worse than reported: whether an overlong field errors out today
depends on where the wrapped value happens to land. For example,
'ffffffff01:0:0:0:0:0' is rejected only because the wrap produces a
negative int, while '100000001:0:0:0:0:0' sails through.

* With the patch, both reported inputs now fail with "invalid input
syntax". Overlong fields that already drew an error, such as
'1ff:0:0:0:0:0', move from "invalid octet value" (22003) to "invalid
input syntax" (22P02), since the format match now fails before the
range check runs.

* Two undocumented forms that were previously accepted with the correct
value become errors: fields zero-padded past two digits
('001:00:2b:01:02:03') and 0x-prefixed fields ('0xff:0:0:0:0:0').
Neither can be produced by macaddr_out, so dumps and restores are
unaffected; the tightening would only bite text held outside the
database (COPY input, application SQL) that relies on those forms.

* macaddr8_in is unaffected — it already uses a hand-rolled parser
rather than sscanf.

* Not addressed here: %2x still accepts an optional sign per C99
('+f:0:0:0:0:0' still parses as 0f:...; '-f:...' is still caught by
the a < 0 range check), and whitespace after a separator is still
skipped. Closing those would mean replacing sscanf with a
hand-rolled parser like macaddr8_in's (which would also fix passing
int * where %x formally wants unsigned int *). That seems like
master-only material, so this patch stays minimal for backpatching.

The patch adds regression tests for the new rejections in both the
colon and dash formats, the surviving "invalid octet value" path, and
soft-error reporting. make check and contrib/btree_gist pass. It
applies to master (00b3e50054); the mac.c hunk applies cleanly to all
of REL_14_STABLE through REL_18_STABLE. One caveat for backpatching
the tests: pg_input_is_valid/pg_input_error_info only exist since v16,
so for 14 and 15 those two statements (and their expected output) need
to be dropped — the plain SELECT casts backpatch verbatim.

Best regards,
Zexin Li

On Fri, Jul 31, 2026 04:54 PM, Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:

> > On 29 Jul 2026, at 02:49, PG Bug reporting form <noreply(at)postgresql(dot)org>
> wrote:
>
> > Happy to provide additional cases or a patch if useful.
>
> Please do, patches are always welcome.
>
> --
> Daniel Gustafsson
>
>
>
>
>
>