pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Cc: Peter Eisentraut <peter@eisentraut.org>
Subject: ACL identifier quoting has some issues
Date: Wed, 02 Jul 2025 17:36:12 -0400
Message-ID: <3792884.1751492172@sss.pgh.pa.us> (raw)

Peter pointed out at [1] that acl.c's getid() behaves oddly
when presented with a string of just two double quotes ("").
If that has any sane interpretation it's as an empty string,
but what you got was a single double quote.

While looking at this I realized that there's another problem:
if the string contains any non-ASCII characters then we will
blindly apply isalnum() to byte(s) with the high bit set,
which will have encoding-dependent, locale-dependent,
and perhaps platform-dependent results.  This could easily
result in putid() electing not to quote some string that,
later in some other environment, getid() will decide is not
a valid identifier, causing dump/reload or similar failures.

So I think we need to apply and back-patch something like
the attached.  Here I've opined that any non-ASCII is safe.
We could invert that and decide that any non-ASCII is unsafe,
but that seems more likely to break existing dumps than this
choice is.

			regards, tom lane

[1] https://www.postgresql.org/message-id/ee96443a-72f3-4a12-8ba7-326069fd1c14%40eisentraut.org

Attachments:

fix-ACL-quoting-logic.patch (text/x-diff, 1.6K) download diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c index ca3c5ee3df3..bd2435198e1 100644 --- a/src/backend/utils/adt/acl.c +++ b/src/backend/utils/adt/acl.c @@ -134,6 +134,16 @@ static AclResult pg_role_aclcheck(Oid role_oid, Oid roleid, AclMode mode); static void RoleMembershipCacheCallback(Datum arg, int cacheid, uint32 hashvalue); +/* + * Test whether an identifier char can be left unquoted in ACLs + */ +static inline bool +is_safe_acl_char(unsigned char c) +{ + /* We don't trust isalnum() to deliver consistent results for non-ASCII */ + return IS_HIGHBIT_SET(c) || isalnum(c) || c == '_'; +} + /* * getid * Consumes the first alphanumeric string (identifier) found in string @@ -162,18 +172,20 @@ getid(const char *s, char *n, Node *escontext) /* This code had better match what putid() does, below */ for (; *s != '\0' && - (isalnum((unsigned char) *s) || - *s == '_' || - *s == '"' || - in_quotes); + (in_quotes || is_safe_acl_char(*s) || *s == '"'); s++) { if (*s == '"') { + if (!in_quotes) + { + in_quotes = true; + continue; + } /* safe to look at next char (could be '\0' though) */ if (*(s + 1) != '"') { - in_quotes = !in_quotes; + in_quotes = false; continue; } /* it's an escaped double quote; skip the escaping char */ @@ -210,7 +222,7 @@ putid(char *p, const char *s) for (src = s; *src; src++) { /* This test had better match what getid() does, above */ - if (!isalnum((unsigned char) *src) && *src != '_') + if (!is_safe_acl_char(*src)) { safe = false; break;

Message-ID: <3792884.1751492172@sss.pgh.pa.us>
Permalink:  ../3792884.1751492172@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/3792884.1751492172@sss.pgh.pa.us

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org, peter@eisentraut.org
  Subject: Re: ACL identifier quoting has some issues
  In-Reply-To: <3792884.1751492172@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox