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

Commit 367e2b2

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 5474ab5 commit 367e2b2

File tree

2 files changed

+51
-36
lines changed

2 files changed

+51
-36
lines changed

‎src/bin/pg_dump/pg_backup_archiver.c‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2841,8 +2841,14 @@ _tocEntryRequired(TocEntry *te, teSection curSection, RestoreOptions *ropt)
28412841
* other information should be generated in binary-upgrade mode (not
28422842
* the actual data).
28432843
*/
2844-
if (!(ropt->binary_upgrade && strcmp(te->desc,"BLOB") == 0) &&
2845-
!(ropt->binary_upgrade && strncmp(te->tag,"LARGE OBJECT ", 13) == 0))
2844+
if (!(ropt->binary_upgrade &&
2845+
(strcmp(te->desc, "BLOB") == 0 ||
2846+
(strcmp(te->desc, "ACL") == 0 &&
2847+
strncmp(te->tag, "LARGE OBJECT ", 13) == 0) ||
2848+
(strcmp(te->desc, "COMMENT") == 0 &&
2849+
strncmp(te->tag, "LARGE OBJECT ", 13) == 0) ||
2850+
(strcmp(te->desc, "SECURITY LABEL") == 0 &&
2851+
strncmp(te->tag, "LARGE OBJECT ", 13) == 0))))
28462852
res = res & REQ_SCHEMA;
28472853
}
28482854

‎src/bin/pg_dump/pg_dump.c‎

Lines changed: 43 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2310,6 +2310,7 @@ dumpDatabase(Archive *fout)
23102310
PQExpBuffer dbQry = createPQExpBuffer();
23112311
PQExpBuffer delQry = createPQExpBuffer();
23122312
PQExpBuffer creaQry = createPQExpBuffer();
2313+
PQExpBuffer labelq = createPQExpBuffer();
23132314
PGconn *conn = GetConnection(fout);
23142315
PGresult *res;
23152316
int i_tableoid,
@@ -2596,16 +2597,20 @@ dumpDatabase(Archive *fout)
25962597
destroyPQExpBuffer(loOutQry);
25972598
}
25982599

2600+
/* Compute correct tag for comments etc */
2601+
appendPQExpBuffer(labelq, "DATABASE %s", fmtId(datname));
2602+
25992603
/* Dump DB comment if any */
26002604
if (fout->remoteVersion >= 80200)
26012605
{
26022606
/*
2603-
* 8.2 keeps comments on shared objects in a shared table, so we
2604-
* cannot use the dumpComment used for other database objects.
2607+
* 8.2 and up keep comments on shared objects in a shared table, so we
2608+
* cannot use the dumpComment() code used for other database objects.
2609+
* Be careful that the ArchiveEntry parameters match that function.
26052610
*/
26062611
char *comment = PQgetvalue(res, 0, PQfnumber(res, "description"));
26072612

2608-
if (comment && strlen(comment))
2613+
if (comment && *comment)
26092614
{
26102615
resetPQExpBuffer(dbQry);
26112616

@@ -2617,17 +2622,17 @@ dumpDatabase(Archive *fout)
26172622
appendStringLiteralAH(dbQry, comment, fout);
26182623
appendPQExpBufferStr(dbQry, ";\n");
26192624

2620-
ArchiveEntry(fout, dbCatId, createDumpId(), datname, NULL, NULL,
2621-
dba, false, "COMMENT", SECTION_NONE,
2625+
ArchiveEntry(fout, nilCatalogId, createDumpId(),
2626+
labelq->data, NULL, NULL, dba,
2627+
false, "COMMENT", SECTION_NONE,
26222628
dbQry->data, "", NULL,
2623-
&dbDumpId, 1, NULL, NULL);
2629+
&(dbDumpId), 1,
2630+
NULL, NULL);
26242631
}
26252632
}
26262633
else
26272634
{
2628-
resetPQExpBuffer(dbQry);
2629-
appendPQExpBuffer(dbQry, "DATABASE %s", fmtId(datname));
2630-
dumpComment(fout, dbQry->data, NULL, "",
2635+
dumpComment(fout, labelq->data, NULL, dba,
26312636
dbCatId, 0, dbDumpId);
26322637
}
26332638

@@ -2643,11 +2648,13 @@ dumpDatabase(Archive *fout)
26432648
shres = ExecuteSqlQuery(fout, seclabelQry->data, PGRES_TUPLES_OK);
26442649
resetPQExpBuffer(seclabelQry);
26452650
emitShSecLabels(conn, shres, seclabelQry, "DATABASE", datname);
2646-
if (strlen(seclabelQry->data))
2647-
ArchiveEntry(fout, dbCatId, createDumpId(), datname, NULL, NULL,
2648-
dba, false, "SECURITY LABEL", SECTION_NONE,
2651+
if (seclabelQry->len > 0)
2652+
ArchiveEntry(fout, nilCatalogId, createDumpId(),
2653+
labelq->data, NULL, NULL, dba,
2654+
false, "SECURITY LABEL", SECTION_NONE,
26492655
seclabelQry->data, "", NULL,
2650-
&dbDumpId, 1, NULL, NULL);
2656+
&(dbDumpId), 1,
2657+
NULL, NULL);
26512658
destroyPQExpBuffer(seclabelQry);
26522659
PQclear(shres);
26532660
}
@@ -2657,6 +2664,7 @@ dumpDatabase(Archive *fout)
26572664
destroyPQExpBuffer(dbQry);
26582665
destroyPQExpBuffer(delQry);
26592666
destroyPQExpBuffer(creaQry);
2667+
destroyPQExpBuffer(labelq);
26602668
}
26612669

26622670
/*
@@ -8618,7 +8626,7 @@ dumpNamespace(Archive *fout, NamespaceInfo *nspinfo)
86188626
nspinfo->dobj.catId, 0, nspinfo->dobj.dumpId);
86198627

86208628
dumpACL(fout, nspinfo->dobj.catId, nspinfo->dobj.dumpId, "SCHEMA",
8621-
qnspname, NULL, nspinfo->dobj.name, NULL,
8629+
qnspname, NULL, labelq->data, NULL,
86228630
nspinfo->rolname, nspinfo->nspacl);
86238631

86248632
free(qnspname);
@@ -8903,7 +8911,7 @@ dumpEnumType(Archive *fout, TypeInfo *tyinfo)
89038911
tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId);
89048912

89058913
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
8906-
qtypname, NULL, tyinfo->dobj.name,
8914+
qtypname, NULL, labelq->data,
89078915
tyinfo->dobj.namespace->dobj.name,
89088916
tyinfo->rolname, tyinfo->typacl);
89098917

@@ -9036,7 +9044,7 @@ dumpRangeType(Archive *fout, TypeInfo *tyinfo)
90369044
tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId);
90379045

90389046
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
9039-
qtypname, NULL, tyinfo->dobj.name,
9047+
qtypname, NULL, labelq->data,
90409048
tyinfo->dobj.namespace->dobj.name,
90419049
tyinfo->rolname, tyinfo->typacl);
90429050

@@ -9106,7 +9114,7 @@ dumpUndefinedType(Archive *fout, TypeInfo *tyinfo)
91069114
tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId);
91079115

91089116
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
9109-
qtypname, NULL, tyinfo->dobj.name,
9117+
qtypname, NULL, labelq->data,
91109118
tyinfo->dobj.namespace->dobj.name,
91119119
tyinfo->rolname, tyinfo->typacl);
91129120

@@ -9495,7 +9503,7 @@ dumpBaseType(Archive *fout, TypeInfo *tyinfo)
94959503
tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId);
94969504

94979505
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
9498-
qtypname, NULL, tyinfo->dobj.name,
9506+
qtypname, NULL, labelq->data,
94999507
tyinfo->dobj.namespace->dobj.name,
95009508
tyinfo->rolname, tyinfo->typacl);
95019509

@@ -9658,7 +9666,7 @@ dumpDomain(Archive *fout, TypeInfo *tyinfo)
96589666
tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId);
96599667

96609668
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
9661-
qtypname, NULL, tyinfo->dobj.name,
9669+
qtypname, NULL, labelq->data,
96629670
tyinfo->dobj.namespace->dobj.name,
96639671
tyinfo->rolname, tyinfo->typacl);
96649672

@@ -9885,7 +9893,7 @@ dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
98859893
tyinfo->dobj.catId, 0, tyinfo->dobj.dumpId);
98869894

98879895
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
9888-
qtypname, NULL, tyinfo->dobj.name,
9896+
qtypname, NULL, labelq->data,
98899897
tyinfo->dobj.namespace->dobj.name,
98909898
tyinfo->rolname, tyinfo->typacl);
98919899

@@ -10197,7 +10205,7 @@ dumpProcLang(Archive *fout, ProcLangInfo *plang)
1019710205

1019810206
if (plang->lanpltrusted)
1019910207
dumpACL(fout, plang->dobj.catId, plang->dobj.dumpId, "LANGUAGE",
10200-
qlanname, NULL, plang->dobj.name,
10208+
qlanname, NULL, labelq->data,
1020110209
lanschema,
1020210210
plang->lanowner, plang->lanacl);
1020310211

@@ -10826,7 +10834,7 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
1082610834
finfo->dobj.catId, 0, finfo->dobj.dumpId);
1082710835

1082810836
dumpACL(fout, finfo->dobj.catId, finfo->dobj.dumpId, "FUNCTION",
10829-
funcsig, NULL, funcsig_tag,
10837+
funcsig, NULL, labelq->data,
1083010838
finfo->dobj.namespace->dobj.name,
1083110839
finfo->rolname, finfo->proacl);
1083210840

@@ -12711,14 +12719,12 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
1271112719
* syntax for zero-argument aggregates and ordered-set aggregates.
1271212720
*/
1271312721
free(aggsig);
12714-
free(aggsig_tag);
1271512722

1271612723
aggsig = format_function_signature(fout, &agginfo->aggfn, true);
12717-
aggsig_tag = format_function_signature(fout, &agginfo->aggfn, false);
1271812724

1271912725
dumpACL(fout, agginfo->aggfn.dobj.catId, agginfo->aggfn.dobj.dumpId,
1272012726
"FUNCTION",
12721-
aggsig, NULL, aggsig_tag,
12727+
aggsig, NULL, labelq->data,
1272212728
agginfo->aggfn.dobj.namespace->dobj.name,
1272312729
agginfo->aggfn.rolname, agginfo->aggfn.proacl);
1272412730

@@ -13152,7 +13158,7 @@ dumpForeignDataWrapper(Archive *fout, FdwInfo *fdwinfo)
1315213158
/* Handle the ACL */
1315313159
dumpACL(fout, fdwinfo->dobj.catId, fdwinfo->dobj.dumpId,
1315413160
"FOREIGN DATA WRAPPER",
13155-
qfdwname, NULL, fdwinfo->dobj.name,
13161+
qfdwname, NULL, labelq->data,
1315613162
NULL, fdwinfo->rolname,
1315713163
fdwinfo->fdwacl);
1315813164

@@ -13245,7 +13251,7 @@ dumpForeignServer(Archive *fout, ForeignServerInfo *srvinfo)
1324513251
/* Handle the ACL */
1324613252
dumpACL(fout, srvinfo->dobj.catId, srvinfo->dobj.dumpId,
1324713253
"FOREIGN SERVER",
13248-
qsrvname, NULL, srvinfo->dobj.name,
13254+
qsrvname, NULL, labelq->data,
1324913255
NULL, srvinfo->rolname,
1325013256
srvinfo->srvacl);
1325113257

@@ -13445,7 +13451,8 @@ dumpDefaultACL(Archive *fout, DefaultACLInfo *daclinfo)
1344513451
* FOREIGN DATA WRAPPER, SERVER, or LARGE OBJECT.
1344613452
* 'name' is the formatted name of the object. Must be quoted etc. already.
1344713453
* 'subname' is the formatted name of the sub-object, if any. Must be quoted.
13448-
* 'tag' is the tag for the archive entry (typ. unquoted name of object).
13454+
* 'tag' is the tag for the archive entry (should be the same tag as would be
13455+
* used for comments etc; for example "TABLE foo").
1344913456
* 'nspname' is the namespace the object is in (NULL if none).
1345013457
* 'owner' is the owner, NULL if there is no owner (for languages).
1345113458
* 'acls' is the string read out of the fooacl system catalog field;
@@ -13523,7 +13530,7 @@ dumpSecLabel(Archive *fout, const char *target,
1352313530
if (dopt->no_security_labels)
1352413531
return;
1352513532

13526-
/* Comments are schema not data ... except blob comments are data */
13533+
/* Security labels are schema not data ... except blob labels are data */
1352713534
if (strncmp(target, "LARGE OBJECT ", 13) != 0)
1352813535
{
1352913536
if (dopt->dataOnly)
@@ -13804,6 +13811,8 @@ dumpTable(Archive *fout, TableInfo *tbinfo)
1380413811
if (tbinfo->dobj.dump && !dopt->dataOnly)
1380513812
{
1380613813
char *namecopy;
13814+
const char *objtype;
13815+
char *acltag;
1380713816

1380813817
if (tbinfo->relkind == RELKIND_SEQUENCE)
1380913818
dumpSequence(fout, tbinfo);
@@ -13812,12 +13821,13 @@ dumpTable(Archive *fout, TableInfo *tbinfo)
1381213821

1381313822
/* Handle the ACL here */
1381413823
namecopy = pg_strdup(fmtId(tbinfo->dobj.name));
13824+
objtype = (tbinfo->relkind == RELKIND_SEQUENCE) ? "SEQUENCE" : "TABLE";
13825+
acltag = psprintf("%s %s", objtype, namecopy);
1381513826
dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId,
13816-
(tbinfo->relkind == RELKIND_SEQUENCE) ? "SEQUENCE" :
13817-
"TABLE",
13818-
namecopy, NULL, tbinfo->dobj.name,
13827+
objtype, namecopy, NULL, acltag,
1381913828
tbinfo->dobj.namespace->dobj.name, tbinfo->rolname,
1382013829
tbinfo->relacl);
13830+
free(acltag);
1382113831

1382213832
/*
1382313833
* Handle column ACLs, if any. Note: we pull these with a separate
@@ -13842,10 +13852,9 @@ dumpTable(Archive *fout, TableInfo *tbinfo)
1384213852
char *attname = PQgetvalue(res, i, 0);
1384313853
char *attacl = PQgetvalue(res, i, 1);
1384413854
char *attnamecopy;
13845-
char *acltag;
1384613855

1384713856
attnamecopy = pg_strdup(fmtId(attname));
13848-
acltag = psprintf("%s.%s", tbinfo->dobj.name, attname);
13857+
acltag = psprintf("COLUMN %s.%s", namecopy, attnamecopy);
1384913858
/* Column's GRANT type is always TABLE */
1385013859
dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId, "TABLE",
1385113860
namecopy, attnamecopy, acltag,

0 commit comments

Comments
 (0)