Skip to content

Commit ad2b7c9

Browse files
nmischnmisch
committed
Fix pg_dump for GRANT OPTION among initial privileges.
The context is an object that no longer bears some aclitem that it bore initially. (A user issued REVOKE or GRANT statements upon the object.) pg_dump is forming SQL to reproduce the object ACL. Since initdb creates no ACL bearing GRANT OPTION, reaching this bug requires an extension where the creation script establishes such an ACL. No PGXN extension does that. If an installation did reach the bug, pg_dump would have omitted a semicolon, causing a REVOKE and the next SQL statement to fail. Separately, since the affected code exists to eliminate an entire aclitem, it wants plain REVOKE, not REVOKE GRANT OPTION FOR. Back-patch to 9.6, where commit 23f34fa first appeared. Discussion: https://postgr.es/m/20210109102423.GA160022@rfd.leadboat.com
1 parent 1a31d8c commit ad2b7c9

File tree

3 files changed

+84
-41
lines changed

3 files changed

+84
-41
lines changed

‎src/bin/pg_dump/dumputils.c

+24-41
Original file line numberDiff line numberDiff line change
@@ -168,48 +168,28 @@ buildACLCommands(const char *name, const char *subname, const char *nspname,
168168
for (i = 0; i < nraclitems; i++)
169169
{
170170
if (!parseAclItem(raclitems[i], type, name, subname, remoteVersion,
171-
grantee, grantor, privs, privswgo))
171+
grantee, grantor, privs, NULL))
172172
{
173173
ok = false;
174174
break;
175175
}
176176

177-
if (privs->len > 0 || privswgo->len > 0)
177+
if (privs->len > 0)
178178
{
179-
if (privs->len > 0)
180-
{
181-
appendPQExpBuffer(firstsql, "%sREVOKE %s ON %s ",
182-
prefix, privs->data, type);
183-
if (nspname && *nspname)
184-
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
185-
appendPQExpBuffer(firstsql, "%s FROM ", name);
186-
if (grantee->len == 0)
187-
appendPQExpBufferStr(firstsql, "PUBLIC;\n");
188-
else if (strncmp(grantee->data, "group ",
189-
strlen("group ")) == 0)
190-
appendPQExpBuffer(firstsql, "GROUP %s;\n",
191-
fmtId(grantee->data + strlen("group ")));
192-
else
193-
appendPQExpBuffer(firstsql, "%s;\n",
194-
fmtId(grantee->data));
195-
}
196-
if (privswgo->len > 0)
197-
{
198-
appendPQExpBuffer(firstsql,
199-
"%sREVOKE GRANT OPTION FOR %s ON %s ",
200-
prefix, privswgo->data, type);
201-
if (nspname && *nspname)
202-
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
203-
appendPQExpBuffer(firstsql, "%s FROM ", name);
204-
if (grantee->len == 0)
205-
appendPQExpBufferStr(firstsql, "PUBLIC");
206-
else if (strncmp(grantee->data, "group ",
207-
strlen("group ")) == 0)
208-
appendPQExpBuffer(firstsql, "GROUP %s",
209-
fmtId(grantee->data + strlen("group ")));
210-
else
211-
appendPQExpBufferStr(firstsql, fmtId(grantee->data));
212-
}
179+
appendPQExpBuffer(firstsql, "%sREVOKE %s ON %s ",
180+
prefix, privs->data, type);
181+
if (nspname && *nspname)
182+
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
183+
appendPQExpBuffer(firstsql, "%s FROM ", name);
184+
if (grantee->len == 0)
185+
appendPQExpBufferStr(firstsql, "PUBLIC;\n");
186+
else if (strncmp(grantee->data, "group ",
187+
strlen("group ")) == 0)
188+
appendPQExpBuffer(firstsql, "GROUP %s;\n",
189+
fmtId(grantee->data + strlen("group ")));
190+
else
191+
appendPQExpBuffer(firstsql, "%s;\n",
192+
fmtId(grantee->data));
213193
}
214194
}
215195
}
@@ -459,10 +439,13 @@ buildDefaultACLCommands(const char *type, const char *nspname,
459439
* (the /grantor part will not be present if pre-7.4 database).
460440
*
461441
* The returned grantee string will be the dequoted username or groupname
462-
* (preceded with "group " in the latter case). The returned grantor is
463-
* the dequoted grantor name or empty. Privilege characters are decoded
464-
* and split between privileges with grant option (privswgo) and without
465-
* (privs).
442+
* (preceded with "group " in the latter case). Note that a grant to PUBLIC
443+
* is represented by an empty grantee string. The returned grantor is the
444+
* dequoted grantor name. Privilege characters are translated to GRANT/REVOKE
445+
* comma-separated privileges lists. If "privswgo" is non-NULL, the result is
446+
* separate lists for privileges with grant option ("privswgo") and without
447+
* ("privs"). Otherwise, "privs" bears every relevant privilege, ignoring the
448+
* grant option distinction.
466449
*
467450
* Note: for cross-version compatibility, it's important to use ALL when
468451
* appropriate.
@@ -512,7 +495,7 @@ parseAclItem(const char *item, const char *type,
512495
do { \
513496
if ((pos = strchr(eqpos + 1, code))) \
514497
{ \
515-
if (*(pos + 1) == '*') \
498+
if (*(pos + 1) == '*' && privswgo != NULL) \
516499
{ \
517500
AddAcl(privswgo, keywd, subname); \
518501
all_without_go = false; \

‎src/test/modules/test_pg_dump/t/001_base.pl

+51
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,57 @@
383383
},
384384
},
385385
386+
'REVOKE ALL ON FUNCTION wgo_then_no_access' => {
387+
create_order => 3,
388+
create_sql => q{
389+
DO $$BEGIN EXECUTE format(
390+
'REVOKE ALL ON FUNCTION wgo_then_no_access()
391+
FROM pg_signal_backend, public, %I',
392+
(SELECT usename
393+
FROM pg_user JOIN pg_proc ON proowner = usesysid
394+
WHERE proname = 'wgo_then_no_access')); END$$;},
395+
regexp => qr/^
396+
\QREVOKE ALL ON FUNCTION public.wgo_then_no_access() FROM PUBLIC;\E
397+
\n\QREVOKE ALL ON FUNCTION public.wgo_then_no_access() FROM \E.*;
398+
\n\QREVOKE ALL ON FUNCTION public.wgo_then_no_access() FROM pg_signal_backend;\E
399+
/xm,
400+
like => {
401+
binary_upgrade => 1,
402+
clean => 1,
403+
clean_if_exists => 1,
404+
createdb => 1,
405+
defaults => 1,
406+
no_owner => 1,
407+
schema_only => 1,
408+
section_pre_data => 1, },
409+
unlike => {
410+
no_privs => 1,
411+
pg_dumpall_globals => 1,
412+
section_post_data => 1, }, },
413+
414+
'REVOKE GRANT OPTION FOR UPDATE ON SEQUENCE wgo_then_regular' => {
415+
create_order => 3,
416+
create_sql => 'REVOKE GRANT OPTION FOR UPDATE ON SEQUENCE
417+
wgo_then_regular FROM pg_signal_backend;',
418+
regexp => qr/^
419+
\QREVOKE ALL ON SEQUENCE public.wgo_then_regular FROM pg_signal_backend;\E
420+
\n\QGRANT SELECT,UPDATE ON SEQUENCE public.wgo_then_regular TO pg_signal_backend;\E
421+
\n\QGRANT USAGE ON SEQUENCE public.wgo_then_regular TO pg_signal_backend WITH GRANT OPTION;\E
422+
/xm,
423+
like => {
424+
binary_upgrade => 1,
425+
clean => 1,
426+
clean_if_exists => 1,
427+
createdb => 1,
428+
defaults => 1,
429+
no_owner => 1,
430+
schema_only => 1,
431+
section_pre_data => 1, },
432+
unlike => {
433+
no_privs => 1,
434+
pg_dumpall_globals => 1,
435+
section_post_data => 1, }, },
436+
386437
'CREATE ACCESS METHOD regress_test_am' => {
387438
regexp => qr/^
388439
\QCREATE ACCESS METHOD regress_test_am TYPE INDEX HANDLER bthandler;\E

‎src/test/modules/test_pg_dump/test_pg_dump--1.0.sql

+9
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,15 @@ GRANT SELECT(col1) ON regress_pg_dump_table TO public;
2828
GRANT SELECT(col2) ON regress_pg_dump_table TO regress_dump_test_role;
2929
REVOKE SELECT(col2) ON regress_pg_dump_table FROM regress_dump_test_role;
3030

31+
CREATE FUNCTION wgo_then_no_access() RETURNS int LANGUAGE SQL AS 'SELECT 1';
32+
GRANT ALL ON FUNCTION wgo_then_no_access()
33+
TO pg_signal_backend WITH GRANT OPTION;
34+
35+
CREATE SEQUENCE wgo_then_regular;
36+
GRANT ALL ON SEQUENCE wgo_then_regular TO pg_signal_backend WITH GRANT OPTION;
37+
REVOKE GRANT OPTION FOR SELECT ON SEQUENCE wgo_then_regular
38+
FROM pg_signal_backend;
39+
3140
CREATE ACCESS METHOD regress_test_am TYPE INDEX HANDLER bthandler;
3241

3342
-- Create a set of objects that are part of the schema created by

0 commit comments

Comments
 (0)