Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 46246fd

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 d66cfe1 commit 46246fd

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
@@ -2943,14 +2943,22 @@ _tocEntryRequired(TocEntry *te, teSection curSection, RestoreOptions *ropt)
29432943
if (ropt->schemaOnly)
29442944
{
29452945
/*
2946+
* The sequence_data option overrides schema-only for SEQUENCE SET.
2947+
*
29462948
* In binary-upgrade mode, even with schema-only set, we do not mask
29472949
* out large objects. Only large object definitions, comments and
29482950
* other information should be generated in binary-upgrade mode (not
29492951
* the actual data).
29502952
*/
29512953
if (!(ropt->sequence_data && strcmp(te->desc, "SEQUENCE SET") == 0) &&
2952-
!(ropt->binary_upgrade && strcmp(te->desc, "BLOB") == 0) &&
2953-
!(ropt->binary_upgrade && strncmp(te->tag, "LARGE OBJECT ", 13) == 0))
2954+
!(ropt->binary_upgrade &&
2955+
(strcmp(te->desc, "BLOB") == 0 ||
2956+
(strcmp(te->desc, "ACL") == 0 &&
2957+
strncmp(te->tag, "LARGE OBJECT ", 13) == 0) ||
2958+
(strcmp(te->desc, "COMMENT") == 0 &&
2959+
strncmp(te->tag, "LARGE OBJECT ", 13) == 0) ||
2960+
(strcmp(te->desc, "SECURITY LABEL") == 0 &&
2961+
strncmp(te->tag, "LARGE OBJECT ", 13) == 0))))
29542962
res = res & REQ_SCHEMA;
29552963
}
29562964

‎src/bin/pg_dump/pg_dump.c

Lines changed: 45 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2453,6 +2453,7 @@ dumpDatabase(Archive *fout)
24532453
PQExpBuffer dbQry = createPQExpBuffer();
24542454
PQExpBuffer delQry = createPQExpBuffer();
24552455
PQExpBuffer creaQry = createPQExpBuffer();
2456+
PQExpBuffer labelq = createPQExpBuffer();
24562457
PGconn *conn = GetConnection(fout);
24572458
PGresult *res;
24582459
int i_tableoid,
@@ -2711,16 +2712,20 @@ dumpDatabase(Archive *fout)
27112712
destroyPQExpBuffer(loOutQry);
27122713
}
27132714

2715+
/* Compute correct tag for comments etc */
2716+
appendPQExpBuffer(labelq, "DATABASE %s", fmtId(datname));
2717+
27142718
/* Dump DB comment if any */
27152719
if (fout->remoteVersion >= 80200)
27162720
{
27172721
/*
2718-
* 8.2 keeps comments on shared objects in a shared table, so we
2719-
* cannot use the dumpComment used for other database objects.
2722+
* 8.2 and up keep comments on shared objects in a shared table, so we
2723+
* cannot use the dumpComment() code used for other database objects.
2724+
* Be careful that the ArchiveEntry parameters match that function.
27202725
*/
27212726
char *comment = PQgetvalue(res, 0, PQfnumber(res, "description"));
27222727

2723-
if (comment && strlen(comment))
2728+
if (comment && *comment)
27242729
{
27252730
resetPQExpBuffer(dbQry);
27262731

@@ -2732,17 +2737,17 @@ dumpDatabase(Archive *fout)
27322737
appendStringLiteralAH(dbQry, comment, fout);
27332738
appendPQExpBufferStr(dbQry, ";\n");
27342739

2735-
ArchiveEntry(fout, dbCatId, createDumpId(), datname, NULL, NULL,
2736-
dba, false, "COMMENT", SECTION_NONE,
2740+
ArchiveEntry(fout, nilCatalogId, createDumpId(),
2741+
labelq->data, NULL, NULL, dba,
2742+
false, "COMMENT", SECTION_NONE,
27372743
dbQry->data, "", NULL,
2738-
&dbDumpId, 1, NULL, NULL);
2744+
&(dbDumpId), 1,
2745+
NULL, NULL);
27392746
}
27402747
}
27412748
else
27422749
{
2743-
resetPQExpBuffer(dbQry);
2744-
appendPQExpBuffer(dbQry, "DATABASE %s", fmtId(datname));
2745-
dumpComment(fout, dbQry->data, NULL, "",
2750+
dumpComment(fout, labelq->data, NULL, dba,
27462751
dbCatId, 0, dbDumpId);
27472752
}
27482753

@@ -2758,11 +2763,13 @@ dumpDatabase(Archive *fout)
27582763
shres = ExecuteSqlQuery(fout, seclabelQry->data, PGRES_TUPLES_OK);
27592764
resetPQExpBuffer(seclabelQry);
27602765
emitShSecLabels(conn, shres, seclabelQry, "DATABASE", datname);
2761-
if (strlen(seclabelQry->data))
2762-
ArchiveEntry(fout, dbCatId, createDumpId(), datname, NULL, NULL,
2763-
dba, false, "SECURITY LABEL", SECTION_NONE,
2766+
if (seclabelQry->len > 0)
2767+
ArchiveEntry(fout, nilCatalogId, createDumpId(),
2768+
labelq->data, NULL, NULL, dba,
2769+
false, "SECURITY LABEL", SECTION_NONE,
27642770
seclabelQry->data, "", NULL,
2765-
&dbDumpId, 1, NULL, NULL);
2771+
&(dbDumpId), 1,
2772+
NULL, NULL);
27662773
destroyPQExpBuffer(seclabelQry);
27672774
PQclear(shres);
27682775
}
@@ -2772,6 +2779,7 @@ dumpDatabase(Archive *fout)
27722779
destroyPQExpBuffer(dbQry);
27732780
destroyPQExpBuffer(delQry);
27742781
destroyPQExpBuffer(creaQry);
2782+
destroyPQExpBuffer(labelq);
27752783
}
27762784

27772785
/*
@@ -9552,7 +9560,7 @@ dumpNamespace(Archive *fout, NamespaceInfo *nspinfo)
95529560

95539561
if (nspinfo->dobj.dump & DUMP_COMPONENT_ACL)
95549562
dumpACL(fout, nspinfo->dobj.catId, nspinfo->dobj.dumpId, "SCHEMA",
9555-
qnspname, NULL, nspinfo->dobj.name, NULL,
9563+
qnspname, NULL, labelq->data, NULL,
95569564
nspinfo->rolname, nspinfo->nspacl, nspinfo->rnspacl,
95579565
nspinfo->initnspacl, nspinfo->initrnspacl);
95589566

@@ -9847,7 +9855,7 @@ dumpEnumType(Archive *fout, TypeInfo *tyinfo)
98479855

98489856
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
98499857
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
9850-
qtypname, NULL, tyinfo->dobj.name,
9858+
qtypname, NULL, labelq->data,
98519859
tyinfo->dobj.namespace->dobj.name,
98529860
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
98539861
tyinfo->inittypacl, tyinfo->initrtypacl);
@@ -9986,7 +9994,7 @@ dumpRangeType(Archive *fout, TypeInfo *tyinfo)
99869994

99879995
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
99889996
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
9989-
qtypname, NULL, tyinfo->dobj.name,
9997+
qtypname, NULL, labelq->data,
99909998
tyinfo->dobj.namespace->dobj.name,
99919999
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
999210000
tyinfo->inittypacl, tyinfo->initrtypacl);
@@ -10062,7 +10070,7 @@ dumpUndefinedType(Archive *fout, TypeInfo *tyinfo)
1006210070

1006310071
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
1006410072
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
10065-
qtypname, NULL, tyinfo->dobj.name,
10073+
qtypname, NULL, labelq->data,
1006610074
tyinfo->dobj.namespace->dobj.name,
1006710075
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
1006810076
tyinfo->inittypacl, tyinfo->initrtypacl);
@@ -10347,7 +10355,7 @@ dumpBaseType(Archive *fout, TypeInfo *tyinfo)
1034710355

1034810356
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
1034910357
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
10350-
qtypname, NULL, tyinfo->dobj.name,
10358+
qtypname, NULL, labelq->data,
1035110359
tyinfo->dobj.namespace->dobj.name,
1035210360
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
1035310361
tyinfo->inittypacl, tyinfo->initrtypacl);
@@ -10515,7 +10523,7 @@ dumpDomain(Archive *fout, TypeInfo *tyinfo)
1051510523

1051610524
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
1051710525
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
10518-
qtypname, NULL, tyinfo->dobj.name,
10526+
qtypname, NULL, labelq->data,
1051910527
tyinfo->dobj.namespace->dobj.name,
1052010528
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
1052110529
tyinfo->inittypacl, tyinfo->initrtypacl);
@@ -10750,7 +10758,7 @@ dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
1075010758

1075110759
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
1075210760
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
10753-
qtypname, NULL, tyinfo->dobj.name,
10761+
qtypname, NULL, labelq->data,
1075410762
tyinfo->dobj.namespace->dobj.name,
1075510763
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
1075610764
tyinfo->inittypacl, tyinfo->initrtypacl);
@@ -11068,7 +11076,7 @@ dumpProcLang(Archive *fout, ProcLangInfo *plang)
1106811076

1106911077
if (plang->lanpltrusted && plang->dobj.dump & DUMP_COMPONENT_ACL)
1107011078
dumpACL(fout, plang->dobj.catId, plang->dobj.dumpId, "LANGUAGE",
11071-
qlanname, NULL, plang->dobj.name,
11079+
qlanname, NULL, labelq->data,
1107211080
lanschema,
1107311081
plang->lanowner, plang->lanacl, plang->rlanacl,
1107411082
plang->initlanacl, plang->initrlanacl);
@@ -11688,7 +11696,7 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
1168811696

1168911697
if (finfo->dobj.dump & DUMP_COMPONENT_ACL)
1169011698
dumpACL(fout, finfo->dobj.catId, finfo->dobj.dumpId, "FUNCTION",
11691-
funcsig, NULL, funcsig_tag,
11699+
funcsig, NULL, labelq->data,
1169211700
finfo->dobj.namespace->dobj.name,
1169311701
finfo->rolname, finfo->proacl, finfo->rproacl,
1169411702
finfo->initproacl, finfo->initrproacl);
@@ -13678,15 +13686,13 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
1367813686
* syntax for zero-argument aggregates and ordered-set aggregates.
1367913687
*/
1368013688
free(aggsig);
13681-
free(aggsig_tag);
1368213689

1368313690
aggsig = format_function_signature(fout, &agginfo->aggfn, true);
13684-
aggsig_tag = format_function_signature(fout, &agginfo->aggfn, false);
1368513691

1368613692
if (agginfo->aggfn.dobj.dump & DUMP_COMPONENT_ACL)
1368713693
dumpACL(fout, agginfo->aggfn.dobj.catId, agginfo->aggfn.dobj.dumpId,
1368813694
"FUNCTION",
13689-
aggsig, NULL, aggsig_tag,
13695+
aggsig, NULL, labelq->data,
1369013696
agginfo->aggfn.dobj.namespace->dobj.name,
1369113697
agginfo->aggfn.rolname, agginfo->aggfn.proacl,
1369213698
agginfo->aggfn.rproacl,
@@ -14132,7 +14138,7 @@ dumpForeignDataWrapper(Archive *fout, FdwInfo *fdwinfo)
1413214138
if (fdwinfo->dobj.dump & DUMP_COMPONENT_ACL)
1413314139
dumpACL(fout, fdwinfo->dobj.catId, fdwinfo->dobj.dumpId,
1413414140
"FOREIGN DATA WRAPPER",
14135-
qfdwname, NULL, fdwinfo->dobj.name,
14141+
qfdwname, NULL, labelq->data,
1413614142
NULL, fdwinfo->rolname,
1413714143
fdwinfo->fdwacl, fdwinfo->rfdwacl,
1413814144
fdwinfo->initfdwacl, fdwinfo->initrfdwacl);
@@ -14229,7 +14235,7 @@ dumpForeignServer(Archive *fout, ForeignServerInfo *srvinfo)
1422914235
if (srvinfo->dobj.dump & DUMP_COMPONENT_ACL)
1423014236
dumpACL(fout, srvinfo->dobj.catId, srvinfo->dobj.dumpId,
1423114237
"FOREIGN SERVER",
14232-
qsrvname, NULL, srvinfo->dobj.name,
14238+
qsrvname, NULL, labelq->data,
1423314239
NULL, srvinfo->rolname,
1423414240
srvinfo->srvacl, srvinfo->rsrvacl,
1423514241
srvinfo->initsrvacl, srvinfo->initrsrvacl);
@@ -14440,7 +14446,8 @@ dumpDefaultACL(Archive *fout, DefaultACLInfo *daclinfo)
1444014446
* FOREIGN DATA WRAPPER, SERVER, or LARGE OBJECT.
1444114447
* 'name' is the formatted name of the object. Must be quoted etc. already.
1444214448
* 'subname' is the formatted name of the sub-object, if any. Must be quoted.
14443-
* 'tag' is the tag for the archive entry (typ. unquoted name of object).
14449+
* 'tag' is the tag for the archive entry (should be the same tag as would be
14450+
* used for comments etc; for example "TABLE foo").
1444414451
* 'nspname' is the namespace the object is in (NULL if none).
1444514452
* 'owner' is the owner, NULL if there is no owner (for languages).
1444614453
* 'acls' contains the ACL string of the object from the appropriate system
@@ -14550,7 +14557,7 @@ dumpSecLabel(Archive *fout, const char *target,
1455014557
if (dopt->no_security_labels)
1455114558
return;
1455214559

14553-
/* Comments are schema not data ... except blob comments are data */
14560+
/* Security labels are schema not data ... except blob labels are data */
1455414561
if (strncmp(target, "LARGE OBJECT ", 13) != 0)
1455514562
{
1455614563
if (dopt->dataOnly)
@@ -14844,13 +14851,18 @@ dumpTable(Archive *fout, TableInfo *tbinfo)
1484414851
/* Handle the ACL here */
1484514852
namecopy = pg_strdup(fmtId(tbinfo->dobj.name));
1484614853
if (tbinfo->dobj.dump & DUMP_COMPONENT_ACL)
14854+
{
14855+
const char *objtype =
14856+
(tbinfo->relkind == RELKIND_SEQUENCE) ? "SEQUENCE" : "TABLE";
14857+
char *acltag = psprintf("%s %s", objtype, namecopy);
14858+
1484714859
dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId,
14848-
(tbinfo->relkind == RELKIND_SEQUENCE) ? "SEQUENCE" :
14849-
"TABLE",
14850-
namecopy, NULL, tbinfo->dobj.name,
14860+
objtype, namecopy, NULL, acltag,
1485114861
tbinfo->dobj.namespace->dobj.name, tbinfo->rolname,
1485214862
tbinfo->relacl, tbinfo->rrelacl,
1485314863
tbinfo->initrelacl, tbinfo->initrrelacl);
14864+
free(acltag);
14865+
}
1485414866

1485514867
/*
1485614868
* Handle column ACLs, if any. Note: we pull these with a separate query
@@ -14934,7 +14946,7 @@ dumpTable(Archive *fout, TableInfo *tbinfo)
1493414946
char *acltag;
1493514947

1493614948
attnamecopy = pg_strdup(fmtId(attname));
14937-
acltag = psprintf("%s.%s", tbinfo->dobj.name, attname);
14949+
acltag = psprintf("COLUMN %s.%s", namecopy, attnamecopy);
1493814950
/* Column's GRANT type is always TABLE */
1493914951
dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId, "TABLE",
1494014952
namecopy, attnamecopy, acltag,

0 commit comments

Comments
 (0)