🌐 AI搜索 & 代理 主页
Skip to content

Commit 2b792ab

Browse files
tglsfdctglsfdc
committed
Make pg_dump's ACL, sec label, and comment entries reliably identifiable.
_tocEntryRequired() expects that it can identify ACL, SECURITY LABEL, and COMMENT TOC entries that are for large objects by seeing whether the tag for them starts with "LARGE OBJECT ". While that works fine for actual large objects, which are indeed tagged that way, it's subject to false positives unless every such entry's tag starts with an appropriate type ID. And in fact it does not work for ACLs, because up to now we customarily tagged those entries with just the bare name of the object. This means that an ACL for an object named "LARGE OBJECT something" would be misclassified as data not schema, with undesirable results in a schema-only or data-only dump --- although pg_upgrade seems unaffected, due to the special case for binary-upgrade mode further down in _tocEntryRequired(). We can fix this by changing all the dumpACL calls to use the label strings already in use for comments and security labels, which do follow the convention of starting with an object type indicator. Well, mostly they follow it. dumpDatabase() got it wrong, using just the bare database name for those purposes, so that a database named "LARGE OBJECT something" would similarly be subject to having its comment or security label dropped or included when not wanted. Bring that into line too. (Note that up to now, database ACLs have not been processed by pg_dump, so that this issue doesn't affect them.) _tocEntryRequired() itself is not free of fault: it was overly liberal about matching object tags to "LARGE OBJECT " in binary-upgrade mode. This looks like it is probably harmless because there would be no data component to strip anyway in that mode, but at best it's trouble waiting to happen, so tighten that up too. The possible misclassification of SECURITY LABEL entries for databases is in principle a security problem, but the opportunities for actual exploits seem too narrow to be interesting. The other cases seem like just bugs, since an object owner can change its ACL or comment for himself, he needn't try to trick someone else into doing it by choosing a strange name. This has been broken since per-large-object TOC entries were introduced in 9.0, so back-patch to all supported branches. Discussion: https://postgr.es/m/21714.1516553459@sss.pgh.pa.us
1 parent 8561e48 commit 2b792ab

File tree

2 files changed

+55
-35
lines changed

2 files changed

+55
-35
lines changed

‎src/bin/pg_dump/pg_backup_archiver.c‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2944,14 +2944,22 @@ _tocEntryRequired(TocEntry *te, teSection curSection, RestoreOptions *ropt)
29442944
if (ropt->schemaOnly)
29452945
{
29462946
/*
2947+
* The sequence_data option overrides schema-only for SEQUENCE SET.
2948+
*
29472949
* In binary-upgrade mode, even with schema-only set, we do not mask
29482950
* out large objects. Only large object definitions, comments and
29492951
* other information should be generated in binary-upgrade mode (not
29502952
* the actual data).
29512953
*/
29522954
if (!(ropt->sequence_data && strcmp(te->desc, "SEQUENCE SET") == 0) &&
2953-
!(ropt->binary_upgrade && strcmp(te->desc, "BLOB") == 0) &&
2954-
!(ropt->binary_upgrade && strncmp(te->tag, "LARGE OBJECT ", 13) == 0))
2955+
!(ropt->binary_upgrade &&
2956+
(strcmp(te->desc, "BLOB") == 0 ||
2957+
(strcmp(te->desc, "ACL") == 0 &&
2958+
strncmp(te->tag, "LARGE OBJECT ", 13) == 0) ||
2959+
(strcmp(te->desc, "COMMENT") == 0 &&
2960+
strncmp(te->tag, "LARGE OBJECT ", 13) == 0) ||
2961+
(strcmp(te->desc, "SECURITY LABEL") == 0 &&
2962+
strncmp(te->tag, "LARGE OBJECT ", 13) == 0))))
29552963
res = res & REQ_SCHEMA;
29562964
}
29572965

‎src/bin/pg_dump/pg_dump.c‎

Lines changed: 45 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2529,6 +2529,7 @@ dumpDatabase(Archive *fout)
25292529
PQExpBuffer dbQry = createPQExpBuffer();
25302530
PQExpBuffer delQry = createPQExpBuffer();
25312531
PQExpBuffer creaQry = createPQExpBuffer();
2532+
PQExpBuffer labelq = createPQExpBuffer();
25322533
PGconn *conn = GetConnection(fout);
25332534
PGresult *res;
25342535
int i_tableoid,
@@ -2787,16 +2788,20 @@ dumpDatabase(Archive *fout)
27872788
destroyPQExpBuffer(loOutQry);
27882789
}
27892790

2791+
/* Compute correct tag for comments etc */
2792+
appendPQExpBuffer(labelq, "DATABASE %s", fmtId(datname));
2793+
27902794
/* Dump DB comment if any */
27912795
if (fout->remoteVersion >= 80200)
27922796
{
27932797
/*
2794-
* 8.2 keeps comments on shared objects in a shared table, so we
2795-
* cannot use the dumpComment used for other database objects.
2798+
* 8.2 and up keep comments on shared objects in a shared table, so we
2799+
* cannot use the dumpComment() code used for other database objects.
2800+
* Be careful that the ArchiveEntry parameters match that function.
27962801
*/
27972802
char *comment = PQgetvalue(res, 0, PQfnumber(res, "description"));
27982803

2799-
if (comment && strlen(comment))
2804+
if (comment && *comment)
28002805
{
28012806
resetPQExpBuffer(dbQry);
28022807

@@ -2808,17 +2813,17 @@ dumpDatabase(Archive *fout)
28082813
appendStringLiteralAH(dbQry, comment, fout);
28092814
appendPQExpBufferStr(dbQry, ";\n");
28102815

2811-
ArchiveEntry(fout, dbCatId, createDumpId(), datname, NULL, NULL,
2812-
dba, false, "COMMENT", SECTION_NONE,
2816+
ArchiveEntry(fout, nilCatalogId, createDumpId(),
2817+
labelq->data, NULL, NULL, dba,
2818+
false, "COMMENT", SECTION_NONE,
28132819
dbQry->data, "", NULL,
2814-
&dbDumpId, 1, NULL, NULL);
2820+
&(dbDumpId), 1,
2821+
NULL, NULL);
28152822
}
28162823
}
28172824
else
28182825
{
2819-
resetPQExpBuffer(dbQry);
2820-
appendPQExpBuffer(dbQry, "DATABASE %s", fmtId(datname));
2821-
dumpComment(fout, dbQry->data, NULL, "",
2826+
dumpComment(fout, labelq->data, NULL, dba,
28222827
dbCatId, 0, dbDumpId);
28232828
}
28242829

@@ -2834,11 +2839,13 @@ dumpDatabase(Archive *fout)
28342839
shres = ExecuteSqlQuery(fout, seclabelQry->data, PGRES_TUPLES_OK);
28352840
resetPQExpBuffer(seclabelQry);
28362841
emitShSecLabels(conn, shres, seclabelQry, "DATABASE", datname);
2837-
if (strlen(seclabelQry->data))
2838-
ArchiveEntry(fout, dbCatId, createDumpId(), datname, NULL, NULL,
2839-
dba, false, "SECURITY LABEL", SECTION_NONE,
2842+
if (seclabelQry->len > 0)
2843+
ArchiveEntry(fout, nilCatalogId, createDumpId(),
2844+
labelq->data, NULL, NULL, dba,
2845+
false, "SECURITY LABEL", SECTION_NONE,
28402846
seclabelQry->data, "", NULL,
2841-
&dbDumpId, 1, NULL, NULL);
2847+
&(dbDumpId), 1,
2848+
NULL, NULL);
28422849
destroyPQExpBuffer(seclabelQry);
28432850
PQclear(shres);
28442851
}
@@ -2848,6 +2855,7 @@ dumpDatabase(Archive *fout)
28482855
destroyPQExpBuffer(dbQry);
28492856
destroyPQExpBuffer(delQry);
28502857
destroyPQExpBuffer(creaQry);
2858+
destroyPQExpBuffer(labelq);
28512859
}
28522860

28532861
/*
@@ -9707,7 +9715,7 @@ dumpNamespace(Archive *fout, NamespaceInfo *nspinfo)
97079715

97089716
if (nspinfo->dobj.dump & DUMP_COMPONENT_ACL)
97099717
dumpACL(fout, nspinfo->dobj.catId, nspinfo->dobj.dumpId, "SCHEMA",
9710-
qnspname, NULL, nspinfo->dobj.name, NULL,
9718+
qnspname, NULL, labelq->data, NULL,
97119719
nspinfo->rolname, nspinfo->nspacl, nspinfo->rnspacl,
97129720
nspinfo->initnspacl, nspinfo->initrnspacl);
97139721

@@ -10003,7 +10011,7 @@ dumpEnumType(Archive *fout, TypeInfo *tyinfo)
1000310011

1000410012
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
1000510013
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
10006-
qtypname, NULL, tyinfo->dobj.name,
10014+
qtypname, NULL, labelq->data,
1000710015
tyinfo->dobj.namespace->dobj.name,
1000810016
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
1000910017
tyinfo->inittypacl, tyinfo->initrtypacl);
@@ -10143,7 +10151,7 @@ dumpRangeType(Archive *fout, TypeInfo *tyinfo)
1014310151

1014410152
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
1014510153
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
10146-
qtypname, NULL, tyinfo->dobj.name,
10154+
qtypname, NULL, labelq->data,
1014710155
tyinfo->dobj.namespace->dobj.name,
1014810156
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
1014910157
tyinfo->inittypacl, tyinfo->initrtypacl);
@@ -10220,7 +10228,7 @@ dumpUndefinedType(Archive *fout, TypeInfo *tyinfo)
1022010228

1022110229
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
1022210230
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
10223-
qtypname, NULL, tyinfo->dobj.name,
10231+
qtypname, NULL, labelq->data,
1022410232
tyinfo->dobj.namespace->dobj.name,
1022510233
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
1022610234
tyinfo->inittypacl, tyinfo->initrtypacl);
@@ -10509,7 +10517,7 @@ dumpBaseType(Archive *fout, TypeInfo *tyinfo)
1050910517

1051010518
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
1051110519
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
10512-
qtypname, NULL, tyinfo->dobj.name,
10520+
qtypname, NULL, labelq->data,
1051310521
tyinfo->dobj.namespace->dobj.name,
1051410522
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
1051510523
tyinfo->inittypacl, tyinfo->initrtypacl);
@@ -10678,7 +10686,7 @@ dumpDomain(Archive *fout, TypeInfo *tyinfo)
1067810686

1067910687
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
1068010688
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
10681-
qtypname, NULL, tyinfo->dobj.name,
10689+
qtypname, NULL, labelq->data,
1068210690
tyinfo->dobj.namespace->dobj.name,
1068310691
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
1068410692
tyinfo->inittypacl, tyinfo->initrtypacl);
@@ -10914,7 +10922,7 @@ dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
1091410922

1091510923
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
1091610924
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
10917-
qtypname, NULL, tyinfo->dobj.name,
10925+
qtypname, NULL, labelq->data,
1091810926
tyinfo->dobj.namespace->dobj.name,
1091910927
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
1092010928
tyinfo->inittypacl, tyinfo->initrtypacl);
@@ -11233,7 +11241,7 @@ dumpProcLang(Archive *fout, ProcLangInfo *plang)
1123311241

1123411242
if (plang->lanpltrusted && plang->dobj.dump & DUMP_COMPONENT_ACL)
1123511243
dumpACL(fout, plang->dobj.catId, plang->dobj.dumpId, "LANGUAGE",
11236-
qlanname, NULL, plang->dobj.name,
11244+
qlanname, NULL, labelq->data,
1123711245
lanschema,
1123811246
plang->lanowner, plang->lanacl, plang->rlanacl,
1123911247
plang->initlanacl, plang->initrlanacl);
@@ -11867,7 +11875,7 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
1186711875

1186811876
if (finfo->dobj.dump & DUMP_COMPONENT_ACL)
1186911877
dumpACL(fout, finfo->dobj.catId, finfo->dobj.dumpId, keyword,
11870-
funcsig, NULL, funcsig_tag,
11878+
funcsig, NULL, labelq->data,
1187111879
finfo->dobj.namespace->dobj.name,
1187211880
finfo->rolname, finfo->proacl, finfo->rproacl,
1187311881
finfo->initproacl, finfo->initrproacl);
@@ -13939,15 +13947,13 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
1393913947
* syntax for zero-argument aggregates and ordered-set aggregates.
1394013948
*/
1394113949
free(aggsig);
13942-
free(aggsig_tag);
1394313950

1394413951
aggsig = format_function_signature(fout, &agginfo->aggfn, true);
13945-
aggsig_tag = format_function_signature(fout, &agginfo->aggfn, false);
1394613952

1394713953
if (agginfo->aggfn.dobj.dump & DUMP_COMPONENT_ACL)
1394813954
dumpACL(fout, agginfo->aggfn.dobj.catId, agginfo->aggfn.dobj.dumpId,
1394913955
"FUNCTION",
13950-
aggsig, NULL, aggsig_tag,
13956+
aggsig, NULL, labelq->data,
1395113957
agginfo->aggfn.dobj.namespace->dobj.name,
1395213958
agginfo->aggfn.rolname, agginfo->aggfn.proacl,
1395313959
agginfo->aggfn.rproacl,
@@ -14393,7 +14399,7 @@ dumpForeignDataWrapper(Archive *fout, FdwInfo *fdwinfo)
1439314399
if (fdwinfo->dobj.dump & DUMP_COMPONENT_ACL)
1439414400
dumpACL(fout, fdwinfo->dobj.catId, fdwinfo->dobj.dumpId,
1439514401
"FOREIGN DATA WRAPPER",
14396-
qfdwname, NULL, fdwinfo->dobj.name,
14402+
qfdwname, NULL, labelq->data,
1439714403
NULL, fdwinfo->rolname,
1439814404
fdwinfo->fdwacl, fdwinfo->rfdwacl,
1439914405
fdwinfo->initfdwacl, fdwinfo->initrfdwacl);
@@ -14490,7 +14496,7 @@ dumpForeignServer(Archive *fout, ForeignServerInfo *srvinfo)
1449014496
if (srvinfo->dobj.dump & DUMP_COMPONENT_ACL)
1449114497
dumpACL(fout, srvinfo->dobj.catId, srvinfo->dobj.dumpId,
1449214498
"FOREIGN SERVER",
14493-
qsrvname, NULL, srvinfo->dobj.name,
14499+
qsrvname, NULL, labelq->data,
1449414500
NULL, srvinfo->rolname,
1449514501
srvinfo->srvacl, srvinfo->rsrvacl,
1449614502
srvinfo->initsrvacl, srvinfo->initrsrvacl);
@@ -14701,7 +14707,8 @@ dumpDefaultACL(Archive *fout, DefaultACLInfo *daclinfo)
1470114707
* FOREIGN DATA WRAPPER, SERVER, or LARGE OBJECT.
1470214708
* 'name' is the formatted name of the object. Must be quoted etc. already.
1470314709
* 'subname' is the formatted name of the sub-object, if any. Must be quoted.
14704-
* 'tag' is the tag for the archive entry (typ. unquoted name of object).
14710+
* 'tag' is the tag for the archive entry (should be the same tag as would be
14711+
* used for comments etc; for example "TABLE foo").
1470514712
* 'nspname' is the namespace the object is in (NULL if none).
1470614713
* 'owner' is the owner, NULL if there is no owner (for languages).
1470714714
* 'acls' contains the ACL string of the object from the appropriate system
@@ -14811,7 +14818,7 @@ dumpSecLabel(Archive *fout, const char *target,
1481114818
if (dopt->no_security_labels)
1481214819
return;
1481314820

14814-
/* Comments are schema not data ... except blob comments are data */
14821+
/* Security labels are schema not data ... except blob labels are data */
1481514822
if (strncmp(target, "LARGE OBJECT ", 13) != 0)
1481614823
{
1481714824
if (dopt->dataOnly)
@@ -15105,13 +15112,18 @@ dumpTable(Archive *fout, TableInfo *tbinfo)
1510515112
/* Handle the ACL here */
1510615113
namecopy = pg_strdup(fmtId(tbinfo->dobj.name));
1510715114
if (tbinfo->dobj.dump & DUMP_COMPONENT_ACL)
15115+
{
15116+
const char *objtype =
15117+
(tbinfo->relkind == RELKIND_SEQUENCE) ? "SEQUENCE" : "TABLE";
15118+
char *acltag = psprintf("%s %s", objtype, namecopy);
15119+
1510815120
dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId,
15109-
(tbinfo->relkind == RELKIND_SEQUENCE) ? "SEQUENCE" :
15110-
"TABLE",
15111-
namecopy, NULL, tbinfo->dobj.name,
15121+
objtype, namecopy, NULL, acltag,
1511215122
tbinfo->dobj.namespace->dobj.name, tbinfo->rolname,
1511315123
tbinfo->relacl, tbinfo->rrelacl,
1511415124
tbinfo->initrelacl, tbinfo->initrrelacl);
15125+
free(acltag);
15126+
}
1511515127

1511615128
/*
1511715129
* Handle column ACLs, if any. Note: we pull these with a separate query
@@ -15195,7 +15207,7 @@ dumpTable(Archive *fout, TableInfo *tbinfo)
1519515207
char *acltag;
1519615208

1519715209
attnamecopy = pg_strdup(fmtId(attname));
15198-
acltag = psprintf("%s.%s", tbinfo->dobj.name, attname);
15210+
acltag = psprintf("COLUMN %s.%s", namecopy, attnamecopy);
1519915211
/* Column's GRANT type is always TABLE */
1520015212
dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId, "TABLE",
1520115213
namecopy, attnamecopy, acltag,

0 commit comments

Comments
 (0)