agora inbox for pgsql-bugs@postgresql.org
help / color / mirror / Atom feedBUG #19583: macaddr input accepts octet fields longer than 8 hex digits
6+ messages / 3 participants
[nested] [flat]
* BUG #19583: macaddr input accepts octet fields longer than 8 hex digits
@ 2026-07-29 00:49 PG Bug reporting form <noreply@postgresql.org>
2026-07-29 12:47 ` Re: BUG #19583: macaddr input accepts octet fields longer than 8 hex digits Daniel Gustafsson <daniel@yesql.se>
0 siblings, 1 reply; 6+ messages in thread
From: PG Bug reporting form @ 2026-07-29 00:49 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: malis@pgrust.com
The following bug has been logged on the website:
Bug reference: 19583
Logged by: Michael Malis
Email address: malis@pgrust.com
PostgreSQL version: 19beta2
Operating system: MacOS
Description:
The macaddr input function accepts colon and dash-separated octet
fields containing more than 8 hexadecimal digits, and stores a value
different from the one entered, with no error.
Steps to reproduce (psql, no ~/.psqlrc, freshly-initialized cluster, default
configuration):
SELECT version();
version
-----------------------------------------------------------------------------------------------------------------------------
PostgreSQL 18.4 (Homebrew) on aarch64-apple-darwin24.6.0, compiled by Apple
clang version 17.0.0 (clang-1700.6.4.2), 64-bit
(1 row)
SELECT '100000001:0:0:0:0:0'::macaddr;
macaddr
-------------------
01:00:00:00:00:00
(1 row)
SELECT '-ffffff01:0:0:0:0:0'::macaddr;
macaddr
-------------------
ff:00:00:00:00:00
(1 row)
Identical results on the Debian-based postgres:18 Docker image.
Expected behavior: both inputs raise an error. The first field of
'100000001:0:0:0:0:0' has the value 0x100000001, which is not a
valid octet (documentation, section 8.9, describes macaddr input as
six groups of two hex digits, with the condensed forms as the only
variants); the second input contains a minus sign, which no macaddr
format includes. For comparison, an out-of-range two-digit-plus
field is rejected as expected:
SELECT '1ff:0:0:0:0:0'::macaddr;
ERROR: invalid octet value in "macaddr" value: "1ff:0:0:0:0:0"
Actual behavior: the overlong field is accepted and the stored octet
is the input value modulo 2^32 (then masked to a byte), i.e. a
different MAC address than the one entered, silently.
Two observations from the source (src/backend/utils/adt/mac.c,
macaddr_in): the first two sscanf formats use unbounded "%x"
conversions, while the five condensed formats in the same function
already use "%2x"; and C99 specifies that %x stores out-of-range
values as the conversion modulo the target width rather than
failing, so the subsequent a > 255 checks test the wrapped value.
The newer macaddr8 type rejects these inputs (its input function
does not use sscanf, per the discussion in
20170312193858.GW9812@tamriel.snowman.net).
Happy to provide additional cases or a patch if useful.
I found this while doing differential testing for pgrust.
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #19583: macaddr input accepts octet fields longer than 8 hex digits
2026-07-29 00:49 BUG #19583: macaddr input accepts octet fields longer than 8 hex digits PG Bug reporting form <noreply@postgresql.org>
@ 2026-07-29 12:47 ` Daniel Gustafsson <daniel@yesql.se>
2026-08-01 01:40 ` Re: BUG #19583: macaddr input accepts octet fields longer than 8 hex digits Zexin Li <lizi.openmind@gmail.com>
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Gustafsson @ 2026-07-29 12:47 UTC (permalink / raw)
To: malis@pgrust.com; pgsql-bugs@lists.postgresql.org
> 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
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #19583: macaddr input accepts octet fields longer than 8 hex digits
2026-07-29 00:49 BUG #19583: macaddr input accepts octet fields longer than 8 hex digits PG Bug reporting form <noreply@postgresql.org>
2026-07-29 12:47 ` Re: BUG #19583: macaddr input accepts octet fields longer than 8 hex digits Daniel Gustafsson <daniel@yesql.se>
@ 2026-08-01 01:40 ` Zexin Li <lizi.openmind@gmail.com>
2026-08-01 19:16 ` Re: BUG #19583: macaddr input accepts octet fields longer than 8 hex digits Daniel Gustafsson <daniel@yesql.se>
0 siblings, 1 reply; 6+ messages in thread
From: Zexin Li @ 2026-08-01 01:40 UTC (permalink / raw)
To: daniel@yesql.se; malis@pgrust.com; +Cc: pgsql-bugs@lists.postgresql.org
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 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
>
>
>
>
>
>
Attachments:
[application/octet-stream] 0001-Reject-overlong-hex-fields-in-macaddr-input.patch (6.3K, ../../CAAP6ZkSeEbhHN+nG_8dduMSCSLZp2HgjUw6PMCFu5eyf+xycuw@mail.gmail.com/3-0001-Reject-overlong-hex-fields-in-macaddr-input.patch)
download | inline diff:
From 6d2b67c004889dbcd2b89e1e888b78487d64011f Mon Sep 17 00:00:00 2001
From: Zexin Li <lizi.openmind@gmail.com>
Date: Fri, 31 Jul 2026 06:21:19 +0000
Subject: [PATCH] Reject overlong hex fields in macaddr input
macaddr_in() parsed the colon- and dash-separated formats with
unbounded sscanf %x conversions, so a field with more than eight hex
digits overflowed the int variable. The C standard leaves such an
overflow undefined; with glibc the value wraps modulo 2^32, so inputs
like '100000001:0:0:0:0:0' were silently accepted and stored as a
different MAC address (01:00:00:00:00:00), bypassing the 0..255 range
check. A signed field such as '-ffffff01:0:0:0:0:0' was likewise
accepted through the same wraparound.
Fix by bounding the conversions with %2x, matching the five condensed
formats in the same function. Overlong fields now fail the format
match and draw "invalid input syntax", deterministically instead of
depending on where the wrapped value happens to land. Overlong fields
that already drew an error before, such as '1ff:0:0:0:0:0', now fail
with SQLSTATE 22P02 (invalid_text_representation) instead of 22003,
since the format match fails before the range check runs.
Two undocumented input forms change behavior as a side effect: fields
zero-padded past two digits ('001:00:2b:01:02:03') and 0x-prefixed
fields ('0xff:0:0:0:0:0') were previously accepted with the correct
value and are now rejected. Neither is ever produced by macaddr
output, so dumps and restores are unaffected.
Bug: #19583
Reported-by: Michael Malis <malis@pgrust.com>
Discussion: https://postgr.es/m/19583-ca7c85d40164f18b@postgresql.org
---
src/backend/utils/adt/mac.c | 4 +--
src/test/regress/expected/macaddr.out | 39 +++++++++++++++++++++++++++
src/test/regress/sql/macaddr.sql | 12 +++++++++
3 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/mac.c b/src/backend/utils/adt/mac.c
index 923c5af5..f243f785 100644
--- a/src/backend/utils/adt/mac.c
+++ b/src/backend/utils/adt/mac.c
@@ -57,10 +57,10 @@ macaddr_in(PG_FUNCTION_ARGS)
/* %1s matches iff there is trailing non-whitespace garbage */
- count = sscanf(str, "%x:%x:%x:%x:%x:%x%1s",
+ count = sscanf(str, "%2x:%2x:%2x:%2x:%2x:%2x%1s",
&a, &b, &c, &d, &e, &f, junk);
if (count != 6)
- count = sscanf(str, "%x-%x-%x-%x-%x-%x%1s",
+ count = sscanf(str, "%2x-%2x-%2x-%2x-%2x-%2x%1s",
&a, &b, &c, &d, &e, &f, junk);
if (count != 6)
count = sscanf(str, "%2x%2x%2x:%2x%2x%2x%1s",
diff --git a/src/test/regress/expected/macaddr.out b/src/test/regress/expected/macaddr.out
index 8d5b2219..ad21db72 100644
--- a/src/test/regress/expected/macaddr.out
+++ b/src/test/regress/expected/macaddr.out
@@ -17,6 +17,32 @@ INSERT INTO macaddr_data VALUES (9, 'not even close'); -- invalid
ERROR: invalid input syntax for type macaddr: "not even close"
LINE 1: INSERT INTO macaddr_data VALUES (9, 'not even close');
^
+-- overlong hex fields must be rejected, not silently wrapped (bug #19583)
+SELECT '100000001:0:0:0:0:0'::macaddr;
+ERROR: invalid input syntax for type macaddr: "100000001:0:0:0:0:0"
+LINE 1: SELECT '100000001:0:0:0:0:0'::macaddr;
+ ^
+SELECT '100000001-0-0-0-0-0'::macaddr;
+ERROR: invalid input syntax for type macaddr: "100000001-0-0-0-0-0"
+LINE 1: SELECT '100000001-0-0-0-0-0'::macaddr;
+ ^
+SELECT '-ffffff01:0:0:0:0:0'::macaddr;
+ERROR: invalid input syntax for type macaddr: "-ffffff01:0:0:0:0:0"
+LINE 1: SELECT '-ffffff01:0:0:0:0:0'::macaddr;
+ ^
+SELECT 'ffffffff01:0:0:0:0:0'::macaddr;
+ERROR: invalid input syntax for type macaddr: "ffffffff01:0:0:0:0:0"
+LINE 1: SELECT 'ffffffff01:0:0:0:0:0'::macaddr;
+ ^
+SELECT '1ff:0:0:0:0:0'::macaddr;
+ERROR: invalid input syntax for type macaddr: "1ff:0:0:0:0:0"
+LINE 1: SELECT '1ff:0:0:0:0:0'::macaddr;
+ ^
+-- an in-range-width negative field is still caught by the octet range check
+SELECT '-f:0:0:0:0:0'::macaddr;
+ERROR: invalid octet value in "macaddr" value: "-f:0:0:0:0:0"
+LINE 1: SELECT '-f:0:0:0:0:0'::macaddr;
+ ^
INSERT INTO macaddr_data VALUES (10, '08:00:2b:01:02:04');
INSERT INTO macaddr_data VALUES (11, '08:00:2b:01:02:02');
INSERT INTO macaddr_data VALUES (12, '08:00:2a:01:02:03');
@@ -183,3 +209,16 @@ SELECT * FROM pg_input_error_info('08:00:2b:01:02:', 'macaddr');
invalid input syntax for type macaddr: "08:00:2b:01:02:" | | | 22P02
(1 row)
+-- overlong hex field (bug #19583)
+SELECT pg_input_is_valid('100000001:0:0:0:0:0', 'macaddr');
+ pg_input_is_valid
+-------------------
+ f
+(1 row)
+
+SELECT * FROM pg_input_error_info('1ff:0:0:0:0:0', 'macaddr');
+ message | detail | hint | sql_error_code
+--------------------------------------------------------+--------+------+----------------
+ invalid input syntax for type macaddr: "1ff:0:0:0:0:0" | | | 22P02
+(1 row)
+
diff --git a/src/test/regress/sql/macaddr.sql b/src/test/regress/sql/macaddr.sql
index f61bdebe..ffc7f816 100644
--- a/src/test/regress/sql/macaddr.sql
+++ b/src/test/regress/sql/macaddr.sql
@@ -14,6 +14,15 @@ INSERT INTO macaddr_data VALUES (7, '08002b010203');
INSERT INTO macaddr_data VALUES (8, '0800:2b01:0203'); -- invalid
INSERT INTO macaddr_data VALUES (9, 'not even close'); -- invalid
+-- overlong hex fields must be rejected, not silently wrapped (bug #19583)
+SELECT '100000001:0:0:0:0:0'::macaddr;
+SELECT '100000001-0-0-0-0-0'::macaddr;
+SELECT '-ffffff01:0:0:0:0:0'::macaddr;
+SELECT 'ffffffff01:0:0:0:0:0'::macaddr;
+SELECT '1ff:0:0:0:0:0'::macaddr;
+-- an in-range-width negative field is still caught by the octet range check
+SELECT '-f:0:0:0:0:0'::macaddr;
+
INSERT INTO macaddr_data VALUES (10, '08:00:2b:01:02:04');
INSERT INTO macaddr_data VALUES (11, '08:00:2b:01:02:02');
INSERT INTO macaddr_data VALUES (12, '08:00:2a:01:02:03');
@@ -47,3 +56,6 @@ SELECT pg_input_is_valid('08:00:2b:01:02:ZZ', 'macaddr');
SELECT * FROM pg_input_error_info('08:00:2b:01:02:ZZ', 'macaddr');
SELECT pg_input_is_valid('08:00:2b:01:02:', 'macaddr');
SELECT * FROM pg_input_error_info('08:00:2b:01:02:', 'macaddr');
+-- overlong hex field (bug #19583)
+SELECT pg_input_is_valid('100000001:0:0:0:0:0', 'macaddr');
+SELECT * FROM pg_input_error_info('1ff:0:0:0:0:0', 'macaddr');
--
2.34.1
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #19583: macaddr input accepts octet fields longer than 8 hex digits
2026-07-29 00:49 BUG #19583: macaddr input accepts octet fields longer than 8 hex digits PG Bug reporting form <noreply@postgresql.org>
2026-07-29 12:47 ` Re: BUG #19583: macaddr input accepts octet fields longer than 8 hex digits Daniel Gustafsson <daniel@yesql.se>
2026-08-01 01:40 ` Re: BUG #19583: macaddr input accepts octet fields longer than 8 hex digits Zexin Li <lizi.openmind@gmail.com>
@ 2026-08-01 19:16 ` Daniel Gustafsson <daniel@yesql.se>
2026-08-04 01:04 ` Re: BUG #19583: macaddr input accepts octet fields longer than 8 hex digits Zexin Li <lizi.openmind@gmail.com>
2026-08-16 23:36 ` Re: BUG #19583: macaddr input accepts octet fields longer than 8 hex digits Zexin Li <lizi.openmind@gmail.com>
0 siblings, 2 replies; 6+ messages in thread
From: Daniel Gustafsson @ 2026-08-01 19:16 UTC (permalink / raw)
To: Zexin Li <lizi.openmind@gmail.com>; +Cc: malis@pgrust.com; pgsql-bugs@lists.postgresql.org
> 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
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #19583: macaddr input accepts octet fields longer than 8 hex digits
2026-07-29 00:49 BUG #19583: macaddr input accepts octet fields longer than 8 hex digits PG Bug reporting form <noreply@postgresql.org>
2026-07-29 12:47 ` Re: BUG #19583: macaddr input accepts octet fields longer than 8 hex digits Daniel Gustafsson <daniel@yesql.se>
2026-08-01 01:40 ` Re: BUG #19583: macaddr input accepts octet fields longer than 8 hex digits Zexin Li <lizi.openmind@gmail.com>
2026-08-01 19:16 ` Re: BUG #19583: macaddr input accepts octet fields longer than 8 hex digits Daniel Gustafsson <daniel@yesql.se>
@ 2026-08-04 01:04 ` Zexin Li <lizi.openmind@gmail.com>
1 sibling, 0 replies; 6+ messages in thread
From: Zexin Li @ 2026-08-04 01:04 UTC (permalink / raw)
To: daniel@yesql.se; +Cc: malis@pgrust.com; pgsql-bugs@lists.postgresql.org
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 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
>
>
>
>
>
>
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #19583: macaddr input accepts octet fields longer than 8 hex digits
2026-07-29 00:49 BUG #19583: macaddr input accepts octet fields longer than 8 hex digits PG Bug reporting form <noreply@postgresql.org>
2026-07-29 12:47 ` Re: BUG #19583: macaddr input accepts octet fields longer than 8 hex digits Daniel Gustafsson <daniel@yesql.se>
2026-08-01 01:40 ` Re: BUG #19583: macaddr input accepts octet fields longer than 8 hex digits Zexin Li <lizi.openmind@gmail.com>
2026-08-01 19:16 ` Re: BUG #19583: macaddr input accepts octet fields longer than 8 hex digits Daniel Gustafsson <daniel@yesql.se>
@ 2026-08-16 23:36 ` Zexin Li <lizi.openmind@gmail.com>
1 sibling, 0 replies; 6+ messages in thread
From: Zexin Li @ 2026-08-16 23:36 UTC (permalink / raw)
To: daniel@yesql.se; +Cc: malis@pgrust.com; pgsql-bugs@lists.postgresql.org
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
Attachments:
[application/octet-stream] v2-0001-Reject-out-of-range-octets-in-macaddr-input.patch (12.6K, ../../CAAP6ZkQbENMVG7jcUbNyFyZLcNinJ7kzWjARXs0pVwsfmVk+pg@mail.gmail.com/3-v2-0001-Reject-out-of-range-octets-in-macaddr-input.patch)
download | inline diff:
From 2fc0352fd10d1542a87fc3d2fb0777d1120a78c9 Mon Sep 17 00:00:00 2001
From: Zexin Li <lizi.openmind@gmail.com>
Date: Tue, 11 Aug 2026 07:44:40 +0000
Subject: [PATCH v2] Reject out-of-range octets in macaddr input
macaddr_in() parsed the colon- and dash-separated notations with
unbounded sscanf %x conversions. A field with more than eight significant hex
digits overflows the int variable, which the C standard leaves
undefined; in practice the value wrapped, so the later 0..255 range
check tested the wrapped result and passed. '100000001:0:0:0:0:0'
was therefore accepted and stored as 01:00:00:00:00:00, an address
other than the one given, and whether an overlong field was rejected
at all depended on where the wrapped value happened to land.
Parse the fields of those two notations with strtol() instead, which
handles an out-of-range value in a defined way on every platform: it
yields the exact value whenever that fits in a long, and LONG_MIN or
LONG_MAX otherwise. Either way the existing octet range check
rejects the field. This follows what from_char_parse_int_len() in
formatting.c does, parsing with strtol() and range-checking the
result.
Because %x is defined in terms of strtoul()'s subject sequence,
strtol() accepts the same field syntax, so every input that is
accepted today and stores the correct value keeps working. That
includes forms which are not documented but have been accepted since
the type was added in 1998: zero-padded fields such as
'001:00:2b:01:02:03', 0x-prefixed fields, signed fields, single-digit
fields, and whitespace after a separator. Both error messages stay
as they are.
The one exception is a bare '0x' field with no hex digit after it:
glibc's sscanf read it as zero, strtol() stops at the '0', and that
acceptance already varied by platform. The colon and dash notations
therefore no longer accept such fields. What happens to an input
containing one is then up to the unchanged condensed templates: most
such inputs draw the syntax error; on glibc, a few dash-separated
ones still match a condensed template and either draw the octet
error or stay accepted with the same stored value; and ones that
used to fail the octet check draw the syntax error now.
The five condensed notations are left alone, as their %2x
conversions cannot overflow.
Bug: #19583
Reported-by: Michael Malis <malis@pgrust.com>
Discussion: https://postgr.es/m/19583-ca7c85d40164f18b@postgresql.org
---
src/backend/utils/adt/mac.c | 120 +++++++++++++++++++-------
src/test/regress/expected/macaddr.out | 75 ++++++++++++++++
src/test/regress/sql/macaddr.sql | 20 +++++
3 files changed, 182 insertions(+), 33 deletions(-)
diff --git a/src/backend/utils/adt/mac.c b/src/backend/utils/adt/mac.c
index 3edaac8bb..65018658d 100644
--- a/src/backend/utils/adt/mac.c
+++ b/src/backend/utils/adt/mac.c
@@ -35,6 +35,51 @@ static int macaddr_cmp_internal(macaddr *a1, macaddr *a2);
static int macaddr_fast_cmp(Datum x, Datum y, SortSupport ssup);
static bool macaddr_abbrev_abort(int memtupcount, SortSupport ssup);
static Datum macaddr_abbrev_convert(Datum original, SortSupport ssup);
+static bool macaddr_parse_octets(const char *str, char separator, long *octets);
+
+/*
+ * Parse the six octets of one of the separator-based notations.
+ *
+ * strtol() accepts the same field syntax as the %x conversions this
+ * replaces, but has defined behavior when a field is out of range: it
+ * yields the exact value whenever that fits in a long, and LONG_MIN or
+ * LONG_MAX otherwise. Either way the caller's 0..255 range check
+ * rejects the field, so no errno test is needed here. One divergence:
+ * strtol() stops at the "0" of a bare "0x" with no hex digit after it,
+ * which glibc's sscanf read as zero (that varied across platforms).
+ *
+ * Returns true if str matched this notation, false if it did not, in
+ * which case the caller falls through to the remaining notations.
+ */
+static bool
+macaddr_parse_octets(const char *str, char separator, long *octets)
+{
+ const char *ptr = str;
+ int i;
+
+ for (i = 0; i < 6; i++)
+ {
+ char *endptr;
+
+ octets[i] = strtol(ptr, &endptr, 16);
+ if (endptr == ptr)
+ return false; /* no digits where an octet was expected */
+ ptr = endptr;
+
+ if (i < 5)
+ {
+ if (*ptr != separator)
+ return false;
+ ptr++;
+ }
+ }
+
+ /* trailing whitespace is accepted, anything else is garbage */
+ while (isspace((unsigned char) *ptr))
+ ptr++;
+
+ return (*ptr == '\0');
+}
/*
* MAC address reader. Accepts several common notations.
@@ -46,6 +91,7 @@ macaddr_in(PG_FUNCTION_ARGS)
char *str = PG_GETARG_CSTRING(0);
Node *escontext = fcinfo->context;
macaddr *result;
+ long octets[6];
int a,
b,
c,
@@ -55,49 +101,57 @@ macaddr_in(PG_FUNCTION_ARGS)
char junk[2];
int count;
- /* %1s matches iff there is trailing non-whitespace garbage */
+ if (!macaddr_parse_octets(str, ':', octets) &&
+ !macaddr_parse_octets(str, '-', octets))
+ {
+ /* %1s matches iff there is trailing non-whitespace garbage */
- count = sscanf(str, "%x:%x:%x:%x:%x:%x%1s",
- &a, &b, &c, &d, &e, &f, junk);
- if (count != 6)
- count = sscanf(str, "%x-%x-%x-%x-%x-%x%1s",
- &a, &b, &c, &d, &e, &f, junk);
- if (count != 6)
count = sscanf(str, "%2x%2x%2x:%2x%2x%2x%1s",
&a, &b, &c, &d, &e, &f, junk);
- if (count != 6)
- count = sscanf(str, "%2x%2x%2x-%2x%2x%2x%1s",
- &a, &b, &c, &d, &e, &f, junk);
- if (count != 6)
- count = sscanf(str, "%2x%2x.%2x%2x.%2x%2x%1s",
- &a, &b, &c, &d, &e, &f, junk);
- if (count != 6)
- count = sscanf(str, "%2x%2x-%2x%2x-%2x%2x%1s",
- &a, &b, &c, &d, &e, &f, junk);
- if (count != 6)
- count = sscanf(str, "%2x%2x%2x%2x%2x%2x%1s",
- &a, &b, &c, &d, &e, &f, junk);
- if (count != 6)
- ereturn(escontext, (Datum) 0,
- (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
- errmsg("invalid input syntax for type %s: \"%s\"", "macaddr",
- str)));
+ if (count != 6)
+ count = sscanf(str, "%2x%2x%2x-%2x%2x%2x%1s",
+ &a, &b, &c, &d, &e, &f, junk);
+ if (count != 6)
+ count = sscanf(str, "%2x%2x.%2x%2x.%2x%2x%1s",
+ &a, &b, &c, &d, &e, &f, junk);
+ if (count != 6)
+ count = sscanf(str, "%2x%2x-%2x%2x-%2x%2x%1s",
+ &a, &b, &c, &d, &e, &f, junk);
+ if (count != 6)
+ count = sscanf(str, "%2x%2x%2x%2x%2x%2x%1s",
+ &a, &b, &c, &d, &e, &f, junk);
+ if (count != 6)
+ ereturn(escontext, (Datum) 0,
+ (errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
+ errmsg("invalid input syntax for type %s: \"%s\"", "macaddr",
+ str)));
+
+ octets[0] = a;
+ octets[1] = b;
+ octets[2] = c;
+ octets[3] = d;
+ octets[4] = e;
+ octets[5] = f;
+ }
- if ((a < 0) || (a > 255) || (b < 0) || (b > 255) ||
- (c < 0) || (c > 255) || (d < 0) || (d > 255) ||
- (e < 0) || (e > 255) || (f < 0) || (f > 255))
+ if ((octets[0] < 0) || (octets[0] > 255) ||
+ (octets[1] < 0) || (octets[1] > 255) ||
+ (octets[2] < 0) || (octets[2] > 255) ||
+ (octets[3] < 0) || (octets[3] > 255) ||
+ (octets[4] < 0) || (octets[4] > 255) ||
+ (octets[5] < 0) || (octets[5] > 255))
ereturn(escontext, (Datum) 0,
(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
errmsg("invalid octet value in \"macaddr\" value: \"%s\"", str)));
result = palloc_object(macaddr);
- result->a = a;
- result->b = b;
- result->c = c;
- result->d = d;
- result->e = e;
- result->f = f;
+ result->a = octets[0];
+ result->b = octets[1];
+ result->c = octets[2];
+ result->d = octets[3];
+ result->e = octets[4];
+ result->f = octets[5];
PG_RETURN_MACADDR_P(result);
}
diff --git a/src/test/regress/expected/macaddr.out b/src/test/regress/expected/macaddr.out
index 8d5b22195..bd904e441 100644
--- a/src/test/regress/expected/macaddr.out
+++ b/src/test/regress/expected/macaddr.out
@@ -17,6 +17,68 @@ INSERT INTO macaddr_data VALUES (9, 'not even close'); -- invalid
ERROR: invalid input syntax for type macaddr: "not even close"
LINE 1: INSERT INTO macaddr_data VALUES (9, 'not even close');
^
+-- Overlong hex fields must be rejected, not silently wrapped (bug #19583)
+SELECT '100000001:0:0:0:0:0'::macaddr;
+ERROR: invalid octet value in "macaddr" value: "100000001:0:0:0:0:0"
+LINE 1: SELECT '100000001:0:0:0:0:0'::macaddr;
+ ^
+SELECT '100000001-0-0-0-0-0'::macaddr;
+ERROR: invalid octet value in "macaddr" value: "100000001-0-0-0-0-0"
+LINE 1: SELECT '100000001-0-0-0-0-0'::macaddr;
+ ^
+SELECT '-ffffff01:0:0:0:0:0'::macaddr;
+ERROR: invalid octet value in "macaddr" value: "-ffffff01:0:0:0:0:0"
+LINE 1: SELECT '-ffffff01:0:0:0:0:0'::macaddr;
+ ^
+SELECT 'ffffffffffffffffff:0:0:0:0:0'::macaddr; -- wider than any long
+ERROR: invalid octet value in "macaddr" value: "ffffffffffffffffff:0:0:0:0:0"
+LINE 1: SELECT 'ffffffffffffffffff:0:0:0:0:0'::macaddr;
+ ^
+-- while the leniency inherited from sscanf's %x conversion is preserved
+SELECT '001:00:2b:01:02:03'::macaddr;
+ macaddr
+-------------------
+ 01:00:2b:01:02:03
+(1 row)
+
+SELECT '0x1:00:2b:01:02:03'::macaddr;
+ macaddr
+-------------------
+ 01:00:2b:01:02:03
+(1 row)
+
+SELECT '+f:00:2b:01:02:03'::macaddr;
+ macaddr
+-------------------
+ 0f:00:2b:01:02:03
+(1 row)
+
+SELECT 'aa: bb:cc:dd:ee:ff'::macaddr;
+ macaddr
+-------------------
+ aa:bb:cc:dd:ee:ff
+(1 row)
+
+SELECT '08:00:2b:01:02:03 '::macaddr;
+ macaddr
+-------------------
+ 08:00:2b:01:02:03
+(1 row)
+
+-- but a bare '0x' field with no hex digit is not valid
+SELECT '0x:00:2b:01:02:03'::macaddr;
+ERROR: invalid input syntax for type macaddr: "0x:00:2b:01:02:03"
+LINE 1: SELECT '0x:00:2b:01:02:03'::macaddr;
+ ^
+-- an octet that fits the field width but is out of range is still caught
+SELECT '1ff:0:0:0:0:0'::macaddr;
+ERROR: invalid octet value in "macaddr" value: "1ff:0:0:0:0:0"
+LINE 1: SELECT '1ff:0:0:0:0:0'::macaddr;
+ ^
+SELECT '-f:0:0:0:0:0'::macaddr;
+ERROR: invalid octet value in "macaddr" value: "-f:0:0:0:0:0"
+LINE 1: SELECT '-f:0:0:0:0:0'::macaddr;
+ ^
INSERT INTO macaddr_data VALUES (10, '08:00:2b:01:02:04');
INSERT INTO macaddr_data VALUES (11, '08:00:2b:01:02:02');
INSERT INTO macaddr_data VALUES (12, '08:00:2a:01:02:03');
@@ -183,3 +245,16 @@ SELECT * FROM pg_input_error_info('08:00:2b:01:02:', 'macaddr');
invalid input syntax for type macaddr: "08:00:2b:01:02:" | | | 22P02
(1 row)
+-- overlong hex field (bug #19583)
+SELECT pg_input_is_valid('100000001:0:0:0:0:0', 'macaddr');
+ pg_input_is_valid
+-------------------
+ f
+(1 row)
+
+SELECT * FROM pg_input_error_info('100000001:0:0:0:0:0', 'macaddr');
+ message | detail | hint | sql_error_code
+---------------------------------------------------------------+--------+------+----------------
+ invalid octet value in "macaddr" value: "100000001:0:0:0:0:0" | | | 22003
+(1 row)
+
diff --git a/src/test/regress/sql/macaddr.sql b/src/test/regress/sql/macaddr.sql
index f61bdebe8..305a2b751 100644
--- a/src/test/regress/sql/macaddr.sql
+++ b/src/test/regress/sql/macaddr.sql
@@ -14,6 +14,23 @@ INSERT INTO macaddr_data VALUES (7, '08002b010203');
INSERT INTO macaddr_data VALUES (8, '0800:2b01:0203'); -- invalid
INSERT INTO macaddr_data VALUES (9, 'not even close'); -- invalid
+-- Overlong hex fields must be rejected, not silently wrapped (bug #19583)
+SELECT '100000001:0:0:0:0:0'::macaddr;
+SELECT '100000001-0-0-0-0-0'::macaddr;
+SELECT '-ffffff01:0:0:0:0:0'::macaddr;
+SELECT 'ffffffffffffffffff:0:0:0:0:0'::macaddr; -- wider than any long
+-- while the leniency inherited from sscanf's %x conversion is preserved
+SELECT '001:00:2b:01:02:03'::macaddr;
+SELECT '0x1:00:2b:01:02:03'::macaddr;
+SELECT '+f:00:2b:01:02:03'::macaddr;
+SELECT 'aa: bb:cc:dd:ee:ff'::macaddr;
+SELECT '08:00:2b:01:02:03 '::macaddr;
+-- but a bare '0x' field with no hex digit is not valid
+SELECT '0x:00:2b:01:02:03'::macaddr;
+-- an octet that fits the field width but is out of range is still caught
+SELECT '1ff:0:0:0:0:0'::macaddr;
+SELECT '-f:0:0:0:0:0'::macaddr;
+
INSERT INTO macaddr_data VALUES (10, '08:00:2b:01:02:04');
INSERT INTO macaddr_data VALUES (11, '08:00:2b:01:02:02');
INSERT INTO macaddr_data VALUES (12, '08:00:2a:01:02:03');
@@ -47,3 +64,6 @@ SELECT pg_input_is_valid('08:00:2b:01:02:ZZ', 'macaddr');
SELECT * FROM pg_input_error_info('08:00:2b:01:02:ZZ', 'macaddr');
SELECT pg_input_is_valid('08:00:2b:01:02:', 'macaddr');
SELECT * FROM pg_input_error_info('08:00:2b:01:02:', 'macaddr');
+-- overlong hex field (bug #19583)
+SELECT pg_input_is_valid('100000001:0:0:0:0:0', 'macaddr');
+SELECT * FROM pg_input_error_info('100000001:0:0:0:0:0', 'macaddr');
--
2.34.1
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2026-08-16 23:36 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-07-29 00:49 BUG #19583: macaddr input accepts octet fields longer than 8 hex digits PG Bug reporting form <noreply@postgresql.org>
2026-07-29 12:47 ` Daniel Gustafsson <daniel@yesql.se>
2026-08-01 01:40 ` Zexin Li <lizi.openmind@gmail.com>
2026-08-01 19:16 ` Daniel Gustafsson <daniel@yesql.se>
2026-08-04 01:04 ` Zexin Li <lizi.openmind@gmail.com>
2026-08-16 23:36 ` Zexin Li <lizi.openmind@gmail.com>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox