Skip to content

Commit

Permalink
Sanitize newlines in object names in "pg_restore -l" output.
Browse files Browse the repository at this point in the history
Commits 89e0bac et al replaced newlines with spaces in object names
printed in SQL comments, but we neglected to consider that the same
names are also printed by "pg_restore -l", and a newline would render
the output unparseable by "pg_restore -L".  Apply the same replacement
in "-l" output.  Since "pg_restore -L" doesn't actually examine any
object names, only the dump ID field that starts each line, this is
enough to fix things for its purposes.

The previous fix was treated as a security issue, and we might have
done that here as well, except that the issue was reported publicly
to start with.  Anyway it's hard to see how this could be exploited
for SQL injection; "pg_restore -L" doesn't do much with the file
except parse it for leading integers.

Per bug #14587 from Milos Urbanek.  Back-patch to all supported versions.

Discussion: https://postgr.es/m/20170310155318.1425.30483@wrigleys.postgresql.org
  • Loading branch information
tglsfdc committed Mar 10, 2017
1 parent 466ee7a commit 88f3743
Showing 1 changed file with 34 additions and 5 deletions.
39 changes: 34 additions & 5 deletions src/bin/pg_dump/pg_backup_archiver.c
Expand Up @@ -1090,7 +1090,8 @@ PrintTOCSummary(Archive *AHX)

ahprintf(AH, ";\n; Archive created at %s\n", stamp_str);
ahprintf(AH, "; dbname: %s\n; TOC Entries: %d\n; Compression: %d\n",
AH->archdbname, AH->tocCount, AH->compression);
replace_line_endings(AH->archdbname),
AH->tocCount, AH->compression);

switch (AH->format)
{
Expand Down Expand Up @@ -1127,10 +1128,37 @@ PrintTOCSummary(Archive *AHX)
curSection = te->section;
if (ropt->verbose ||
(_tocEntryRequired(te, curSection, ropt) & (REQ_SCHEMA | REQ_DATA)) != 0)
{
char *sanitized_name;
char *sanitized_schema;
char *sanitized_owner;

/*
* As in _printTocEntry(), sanitize strings that might contain
* newlines, to ensure that each logical output line is in fact
* one physical output line. This prevents confusion when the
* file is read by "pg_restore -L". Note that we currently don't
* bother to quote names, meaning that the name fields aren't
* automatically parseable. "pg_restore -L" doesn't care because
* it only examines the dumpId field, but someday we might want to
* try harder.
*/
sanitized_name = replace_line_endings(te->tag);
if (te->namespace)
sanitized_schema = replace_line_endings(te->namespace);
else
sanitized_schema = pg_strdup("-");
sanitized_owner = replace_line_endings(te->owner);

ahprintf(AH, "%d; %u %u %s %s %s %s\n", te->dumpId,
te->catalogId.tableoid, te->catalogId.oid,
te->desc, te->namespace ? te->namespace : "-",
te->tag, te->owner);
te->desc, sanitized_schema, sanitized_name,
sanitized_owner);

free(sanitized_name);
free(sanitized_schema);
free(sanitized_owner);
}
if (ropt->verbose && te->nDeps > 0)
{
int i;
Expand Down Expand Up @@ -3432,8 +3460,9 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool isData, bool acl_pass)
}

/*
* Sanitize a string to be included in an SQL comment, by replacing any
* newlines with spaces.
* Sanitize a string to be included in an SQL comment or TOC listing,
* by replacing any newlines with spaces.
* The result is a freshly malloc'd string.
*/
static char *
replace_line_endings(const char *str)
Expand Down

0 comments on commit 88f3743

Please sign in to comment.