| From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
|---|---|
| To: | Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)org> |
| Cc: | Aleksander Alekseev <aleksander(at)tigerdata(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Andrey Borodin <x4mmm(at)yandex-team(dot)ru> |
| Subject: | Re: Add uuid_to_base32hex() and base32hex_to_uuid() built-in functions |
| Date: | 2026-03-19 21:33:36 |
| Message-ID: | CAD21AoA7UMWG8YO2Edf1UWSdtaof6gOnF98bJGoz1z1JBzozuQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Mar 19, 2026 at 7:36 AM Dagfinn Ilmari Mannsåker
<ilmari(at)ilmari(dot)org> wrote:
>
> Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> writes:
>
> > I've made some minor changes to both patches (e.g., rewording the
> > documentation changes and commit messages etc), and attached the
> > updated patches.
> >
> > I'm going to push these patches unless there is no further comment.
>
> Just one minor nitpick on my patch, which is that it should use
> palloc_object(), which I wasn't aware of when I wrote it originally.
>
> > diff --git a/src/backend/utils/adt/bytea.c b/src/backend/utils/adt/bytea.c
> > index fd7662d41ee..4dc83671aa5 100644
> > --- a/src/backend/utils/adt/bytea.c
> > +++ b/src/backend/utils/adt/bytea.c
> [...]
> > + if (len != UUID_LEN)
> > + ereport(ERROR,
> > + (errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
> > + errmsg("invalid input length for type %s", "uuid"),
> > + errdetail("Expected %d bytes, got %d.", UUID_LEN, len)));
> > +
> > + uuid = (pg_uuid_t *) palloc(sizeof(pg_uuid_t));
>
> this should be:
>
> + uuid = palloc_object(pg_uuid_t);
>
> > + memcpy(uuid->data, VARDATA_ANY(v), UUID_LEN);
> > + PG_RETURN_UUID_P(uuid);
> > +}
> > +
Good catch. I've pushed the 0001 patch after incorporating this change.
For 0002 patch, I don't push it yet as I've found a bug in the
decoding code during the self-review:
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid symbol \"%.*s\" found while
decoding base32hex sequence",
+ pg_mblen((const char *) &c), (const char *) &c)));
We should not use pg_mblen() anymore (c.f., CVE-2026-2006). And since
'c' is just a single byte on the stack, it leads to a buffer over-read
if the invalid character is a multi-byte character.
Also, a small nitpick is that we can use uint32 instead of uint64 for
'bits_buffer'. I've attached the updated patch as well as the
difference from the previous version.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
| Attachment | Content-Type | Size |
|---|---|---|
| fix_issue_masahiko.patch | text/x-patch | 2.6 KB |
| v8-0001-Add-base32hex-support-to-encode-and-decode-functi.patch | text/x-patch | 15.9 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Robert Haas | 2026-03-19 21:33:37 | Re: pg_plan_advice |
| Previous Message | Zsolt Parragi | 2026-03-19 21:32:21 | Re: pg_get__*_ddl consolidation |