Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitccacaf4

Browse files
committed
Fix inconsistent quoting of role names in ACLs.
getid() and putid(), which parse and deparse role names within ACLinput/output, applied isalnum() to see if a character within a rolename requires quoting. They did this even for non-ASCII characters,which is problematic because the results would depend on encoding,locale, and perhaps even platform. So it's possible that putid()could elect not to quote some string that, later in some otherenvironment, getid() will decide is not a valid identifier, causingdump/reload or similar failures.To fix this in a way that won't risk interoperability problemswith unpatched versions, make getid() treat any non-ASCII as alegitimate identifier character (hence not requiring quotes),while making putid() treat any non-ASCII as requiring quoting.We could remove the resulting excess quoting once we feel thatno unpatched servers remain in the wild, but that'll be years.A lesser problem is that getid() did the wrong thing with an inputconsisting of just two double quotes (""). That has to represent anempty string, but getid() read it as a single double quote instead.The case cannot arise in the normal course of events, since we don'tallow empty-string role names. But let's fix it while we're here.Although we've not heard field reports of problems with non-ASCIIrole names, there's clearly a hazard there, so back-patch to allsupported versions.Reported-by: Peter Eisentraut <peter@eisentraut.org>Author: Tom Lane <tgl@sss.pgh.pa.us>Discussion:https://postgr.es/m/3792884.1751492172@sss.pgh.pa.usBackpatch-through: 13
1 parent3d23f68 commitccacaf4

File tree

3 files changed

+53
-8
lines changed

3 files changed

+53
-8
lines changed

‎src/backend/utils/adt/acl.c‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,22 @@ static AclResult pg_role_aclcheck(Oid role_oid, Oid roleid, AclMode mode);
134134
staticvoidRoleMembershipCacheCallback(Datumarg,intcacheid,uint32hashvalue);
135135

136136

137+
/*
138+
* Test whether an identifier char can be left unquoted in ACLs.
139+
*
140+
* Formerly, we used isalnum() even on non-ASCII characters, resulting in
141+
* unportable behavior. To ensure dump compatibility with old versions,
142+
* we now treat high-bit-set characters as always requiring quoting during
143+
* putid(), but getid() will always accept them without quotes.
144+
*/
145+
staticinlinebool
146+
is_safe_acl_char(unsignedcharc,boolis_getid)
147+
{
148+
if (IS_HIGHBIT_SET(c))
149+
returnis_getid;
150+
returnisalnum(c)||c=='_';
151+
}
152+
137153
/*
138154
* getid
139155
*Consumes the first alphanumeric string (identifier) found in string
@@ -159,21 +175,22 @@ getid(const char *s, char *n, Node *escontext)
159175

160176
while (isspace((unsignedchar)*s))
161177
s++;
162-
/* This code had better match what putid() does, below */
163178
for (;
164179
*s!='\0'&&
165-
(isalnum((unsignedchar)*s)||
166-
*s=='_'||
167-
*s=='"'||
168-
in_quotes);
180+
(in_quotes||*s=='"'||is_safe_acl_char(*s, true));
169181
s++)
170182
{
171183
if (*s=='"')
172184
{
185+
if (!in_quotes)
186+
{
187+
in_quotes= true;
188+
continue;
189+
}
173190
/* safe to look at next char (could be '\0' though) */
174191
if (*(s+1)!='"')
175192
{
176-
in_quotes=!in_quotes;
193+
in_quotes=false;
177194
continue;
178195
}
179196
/* it's an escaped double quote; skip the escaping char */
@@ -207,10 +224,10 @@ putid(char *p, const char *s)
207224
constchar*src;
208225
boolsafe= true;
209226

227+
/* Detect whether we need to use double quotes */
210228
for (src=s;*src;src++)
211229
{
212-
/* This test had better match what getid() does, above */
213-
if (!isalnum((unsignedchar)*src)&&*src!='_')
230+
if (!is_safe_acl_char(*src, false))
214231
{
215232
safe= false;
216233
break;

‎src/test/regress/expected/privileges.out‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2568,6 +2568,26 @@ SELECT makeaclitem('regress_priv_user1'::regrole, 'regress_priv_user2'::regrole,
25682568
SELECT makeaclitem('regress_priv_user1'::regrole, 'regress_priv_user2'::regrole,
25692569
'SELECT, fake_privilege', FALSE); -- error
25702570
ERROR: unrecognized privilege type: "fake_privilege"
2571+
-- Test quoting and dequoting of user names in ACLs
2572+
CREATE ROLE "regress_""quoted";
2573+
SELECT makeaclitem('regress_"quoted'::regrole, 'regress_"quoted'::regrole,
2574+
'SELECT', TRUE);
2575+
makeaclitem
2576+
------------------------------------------
2577+
"regress_""quoted"=r*/"regress_""quoted"
2578+
(1 row)
2579+
2580+
SELECT '"regress_""quoted"=r*/"regress_""quoted"'::aclitem;
2581+
aclitem
2582+
------------------------------------------
2583+
"regress_""quoted"=r*/"regress_""quoted"
2584+
(1 row)
2585+
2586+
SELECT '""=r*/""'::aclitem; -- used to be misparsed as """"
2587+
ERROR: a name must follow the "/" sign
2588+
LINE 1: SELECT '""=r*/""'::aclitem;
2589+
^
2590+
DROP ROLE "regress_""quoted";
25712591
-- Test non-throwing aclitem I/O
25722592
SELECT pg_input_is_valid('regress_priv_user1=r/regress_priv_user2', 'aclitem');
25732593
pg_input_is_valid

‎src/test/regress/sql/privileges.sql‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1544,6 +1544,14 @@ SELECT makeaclitem('regress_priv_user1'::regrole, 'regress_priv_user2'::regrole,
15441544
SELECT makeaclitem('regress_priv_user1'::regrole,'regress_priv_user2'::regrole,
15451545
'SELECT, fake_privilege', FALSE);-- error
15461546

1547+
-- Test quoting and dequoting of user names in ACLs
1548+
CREATE ROLE"regress_""quoted";
1549+
SELECT makeaclitem('regress_"quoted'::regrole,'regress_"quoted'::regrole,
1550+
'SELECT', TRUE);
1551+
SELECT'"regress_""quoted"=r*/"regress_""quoted"'::aclitem;
1552+
SELECT'""=r*/""'::aclitem;-- used to be misparsed as """"
1553+
DROP ROLE"regress_""quoted";
1554+
15471555
-- Test non-throwing aclitem I/O
15481556
SELECT pg_input_is_valid('regress_priv_user1=r/regress_priv_user2','aclitem');
15491557
SELECT pg_input_is_valid('regress_priv_user1=r/','aclitem');

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp