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

On 29 Jul 2026, at 02:49, PG Bug reporting form <noreply@postgresql.org> wrote:

Happy to provide additional cases or a patch if useful.

Please do, patches are always welcome.

--
Daniel Gustafsson

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@yesql.se> wrote:

On 1 Aug 2026, at 03:40, Zexin Li <lizi.openmind@gmail.com> wrote:

Thanks for the patch!

* 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.

This doesn't seem like something we can backpatch though. Our tools might not
produce them, but they may exist in queries and 3rd party tooling which should
not break in a minor rev. I'm not convinced that either of these should be
promoted to errors even in a major rev.

--
Daniel Gustafsson

On Fri, Aug 1, 2026, Daniel Gustafsson wrote:

This doesn't seem like something we can backpatch though. Our tools might
not produce them, but they may exist in queries and 3rd party tooling

which

should not break in a minor rev. I'm not convinced that either of these
should be promoted to errors even in a major rev.

Fair point, thanks for looking at it. To put the question on firmer
footing I tried to check where MAC text parsing actually lives, what
both types accept today, and whether there is an existing convention
in the tree to follow. The short version: the leniency looks
accidental to me, but I agree it may well be load-bearing, so neither
option below would reject those forms on any branch.

1. Where MAC text is parsed

As far as I can see there are exactly two cstring input paths in
pg_proc: macaddr_in() in src/backend/utils/adt/mac.c and
macaddr8_in() in mac8.c. macaddr_recv()/macaddr8_recv() are binary,
and btree_gist/btree_gin/BRIN operate on already-parsed values.

macaddr_in is a cascade of seven sscanf templates; the two
separator-based ones use unbounded %x (where the reported bug lives),
the five condensed ones use %2x. It has two error sites: "invalid
input syntax" (22P02) when all templates fail, and "invalid octet
value" (22003) from the 0..255 range check. The closest thing to a
statement of intent in the file is the comment "Accepts several
common notations."

macaddr8_in is a hand-rolled parser (hex2_to_uchar): exactly two hex
digits per byte, an optional but consistent separator (:, - or .)
after any byte, 6 or 8 bytes total, and a single failure exit
(22P02). No sscanf, so overflow cannot arise there by construction.

2. What the two types accept today (tested on 16.13; the relevant
code is unchanged on master)

input | macaddr | macaddr8
-------------------------+-----------------------+---------
'100000001:0:0:0:0:0' | 01:00:00:00:00:00 (!) | error
'0ff:00:2b:01:02:03' | ff:00:2b:01:02:03 | error
'a:b:c:d:e:f' | 0a:0b:0c:0d:0e:0f | error
'+f:00:2b:01:02:03' | 0f:00:2b:01:02:03 | error
'0x1:00:2b:01:02:03' | 01:00:2b:01:02:03 | error
'aa: bb:cc:dd:ee:ff' | aa:bb:cc:dd:ee:ff | error
'aa.bb.cc.dd.ee.ff' | error | accepted
'aa:bbcc:dd:ee:ff' | error | accepted
'1ff:0:0:0:0:0' | error, 22003 | error, 22P02

(macaddr8 was fed the corresponding 6/8-byte forms, e.g.
'0ff:00:2b:01:02:03:04:05' and 'aa.bb.cc.dd.ee.ff.00.11'. The (!)
row is the reported bug: the stored value is not the value
entered, and neither error site fires.)

The lenient field parsing comes from sscanf's conversion semantics —
%x follows strtoul's subject-sequence rules. As far as I can find
it is not documented (datatype.sgml describes the seven notations
and case-insensitivity only), and no regression test exercises it (I
looked through the macaddr tests in src/test/regress and in
contrib's btree_gist/btree_gin); it appears to go back to the
original 1998 commit (2d69fd90b9), and I did not find a later commit
blessing it — though you may well know of history that never made it
into the tree. I also looked for an existing convention to borrow,
but practice nearby isn't uniform: the integer types deliberately
accept signs, leading zeros and (since v16) 0x prefixes, and inet
accepts '192.168.001.001', while macaddr8 went the strict way.
Either way, it is nothing a bugfix should change as a side effect —
which is what v1 did, and what the options below avoid.

3. Two options for a v2, both fixing the silent wraparound while
keeping every accepted row above working

a. Widen the octet variables to unsigned 64 bit. A small, mostly
mechanical diff, though it touches all seven templates (%x/%2x
become %llx/%2llx) and the now-dead a < 0 half of the range
check. The reported inputs then fail the existing "invalid octet
value" check instead of wrapping. The remaining wart is that
out-of-range sscanf conversion is still undefined behavior, so
the cliff only moves from 9 hex digits out to 17, where glibc
would wrap silently again ('100000000000000ff' -> ff).

b. Parse the fields of the two %x templates with strtoul directly.
Since %x follows strtoul's subject-sequence rules anyway, the
accepted rows above keep working, and strtoul's overflow behavior
is defined everywhere (ULONG_MAX plus ERANGE, whatever the width
of long), so the undefined behavior goes away entirely rather
than moving further out. Overlong fields fall into the existing
"invalid octet value" error, same as '1ff:...' today, so error
texts and SQLSTATEs stay as they are. A somewhat larger diff
than (a): a small helper loop replacing the two sscanf calls.
(One micro-exception I'm aware of: a bare '0x' field with no hex
digit after it, which glibc's sscanf happens to read as zero
today, would become a syntax error; that acceptance already
varies by platform.)

These are the two approaches I
could come up with — if there is a better one, please do suggest it.

Happy to send a patch once there's agreement on the direction.

Best regards,
Zexin Li

On Tue, Aug 04, 2026 09:57 AM, Daniel Gustafsson <daniel@yesql.se> wrote:

On Aug 4, 2026, Zexin Li wrote:

For the problem at hand I lean towards (b), since it removes the
undefined behavior instead of relocating it, but (a) is the smaller
change and I'd be fine with either.

I tried implementing (b); patch attached.

The colon- and dash-separated formats are now parsed by a small
helper that reads each field with strtol() and hands the six values
to the existing 0..255 range check. The five condensed %2x formats
are untouched. Since %x is defined in terms of strtoul()'s subject
sequence, strtol() accepts the same field syntax, so the forms v1
rejected as a side effect are still accepted this time. Each line
shows the old behavior, then the new one:

'001:00:2b:01:02:03': accepted; unchanged
'0x1:00:2b:01:02:03': accepted; unchanged
'+f:00:2b:01:02:03': accepted; unchanged
'a:b:c:d:e:f': accepted; unchanged
'aa: bb:cc:dd:ee:ff': accepted; unchanged
'1ff:0:0:0:0:0': octet error (22003); unchanged
'-f:0:0:0:0:0': octet error (22003); unchanged
'100000001:0:0:0:0:0': stored 01:00:00:00:00:00; now the octet error
'-ffffff01:0:0:0:0:0': stored ff:00:00:00:00:00; now the octet error
'0x:00:2b:01:02:03': accepted on glibc; now the syntax error (22P02)

A field whose value does not fit an octet now reliably draws the
existing "invalid octet value" error, the same one '1ff:...' gets
today; both error texts stay as they are. No errno check is needed:
for such a field, strtol() returns either the exact value (when it
fits in a long) or LONG_MIN/LONG_MAX (when it does not), and both
fail the existing range check. This follows what
from_char_parse_int_len() in formatting.c does, parsing with
strtol() and range-checking the result. I used strtol() rather than
strtoul() so that a negative field is still seen as a negative
value by the (a < 0) half of the existing check, as before.

The remaining difference is the bare-'0x' case from my previous
mail: a '0x' field with no hex digit after it, which glibc's sscanf
read as zero while strtol() stops at the '0' (that acceptance
already varied by platform). The two rewritten formats no longer
accept such a field; that part is strtol()'s standard
subject-sequence behavior, not a glibc detail. What happens to the
input then depends on the unchanged condensed templates: most such
inputs draw the syntax error ('0x:00:2b:01:02:03' above); on glibc,
a few dash-separated ones still match a condensed template and
either draw the octet error ('0x-1-0-0-0-0') or stay accepted with
the same stored value ('0x-0-0-0-0-0'); and ones that used to fail
the octet check, like '0x:1ff:0:0:0:0', draw the syntax error now.
I documented this in the commit message rather than try to unify
the outcomes, which would have meant touching the condensed
templates too.

I also compared the old and the new parsing with a differential
harness over about 3.7 million generated inputs (field shapes x
separators x whitespace placements, plus random strings). The only
divergences are the fields that used to wrap, now rejected, and the
bare-'0x' class above; no input is newly accepted, and no accepted
input changes its stored value.

Besides the rejection cases, which fail without the code change,
the added regression tests also pin the accepted forms, so a later
change that stops accepting one of them would show up.

I'd appreciate any feedback.

Best regards,
Zexin Li