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

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(at)yesql(dot)se> wrote:

> > On 1 Aug 2026, at 03:40, Zexin Li <lizi(dot)openmind(at)gmail(dot)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
>
>
>
>
>
>