Skip to content

Commit 13a67ce

Browse files
nmischnmisch
committed
Convert newlines to spaces in names written in v11+ pg_dump comments.
Maliciously-crafted object names could achieve SQL injection during restore. CVE-2012-0868 fixed this class of problem at the time, but later work reintroduced three cases. Commit bc8cd50 (back-patched to v11+ in 2023-05 releases) introduced the pg_dump case. Commit 6cbdbd9 (v12+) introduced the two pg_dumpall cases. Move sanitize_line(), unchanged, to dumputils.c so pg_dumpall has access to it in all supported versions. Back-patch to v13 (all supported versions). Reviewed-by: Robert Haas <robertmhaas@gmail.com> Reviewed-by: Nathan Bossart <nathandbossart@gmail.com> Backpatch-through: 13 Security: CVE-2025-8715
1 parent 605fdb9 commit 13a67ce

File tree

7 files changed

+90
-40
lines changed

7 files changed

+90
-40
lines changed

‎src/bin/pg_dump/dumputils.c

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,43 @@ static void AddAcl(PQExpBuffer aclbuf, const char *keyword,
3131
const char *subname);
3232

3333

34+
/*
35+
* Sanitize a string to be included in an SQL comment or TOC listing, by
36+
* replacing any newlines with spaces. This ensures each logical output line
37+
* is in fact one physical output line, to prevent corruption of the dump
38+
* (which could, in the worst case, present an SQL injection vulnerability
39+
* if someone were to incautiously load a dump containing objects with
40+
* maliciously crafted names).
41+
*
42+
* The result is a freshly malloc'd string. If the input string is NULL,
43+
* return a malloc'ed empty string, unless want_hyphen, in which case return a
44+
* malloc'ed hyphen.
45+
*
46+
* Note that we currently don't bother to quote names, meaning that the name
47+
* fields aren't automatically parseable. "pg_restore -L" doesn't care because
48+
* it only examines the dumpId field, but someday we might want to try harder.
49+
*/
50+
char *
51+
sanitize_line(const char *str, bool want_hyphen)
52+
{
53+
char *result;
54+
char *s;
55+
56+
if (!str)
57+
return pg_strdup(want_hyphen ? "-" : "");
58+
59+
result = pg_strdup(str);
60+
61+
for (s = result; *s != '\0'; s++)
62+
{
63+
if (*s == '\n' || *s == '\r')
64+
*s = ' ';
65+
}
66+
67+
return result;
68+
}
69+
70+
3471
/*
3572
* Build GRANT/REVOKE command(s) for an object.
3673
*

‎src/bin/pg_dump/dumputils.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#endif
3737

3838

39+
extern char *sanitize_line(const char *str, bool want_hyphen);
3940
extern bool buildACLCommands(const char *name, const char *subname, const char *nspname,
4041
const char *type, const char *acls, const char *baseacls,
4142
const char *owner, const char *prefix, int remoteVersion,

‎src/bin/pg_dump/pg_backup_archiver.c

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ static ArchiveHandle *_allocAH(const char *FileSpec, const ArchiveFormat fmt,
5757
DataDirSyncMethod sync_method);
5858
static void _getObjectDescription(PQExpBuffer buf, const TocEntry *te);
5959
static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, const char *pfx);
60-
static char *sanitize_line(const char *str, bool want_hyphen);
6160
static void _doSetFixedOutputState(ArchiveHandle *AH);
6261
static void _doSetSessionAuth(ArchiveHandle *AH, const char *user);
6362
static void _reconnectToDB(ArchiveHandle *AH, const char *dbname);
@@ -4035,42 +4034,6 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, const char *pfx)
40354034
}
40364035
}
40374036

4038-
/*
4039-
* Sanitize a string to be included in an SQL comment or TOC listing, by
4040-
* replacing any newlines with spaces. This ensures each logical output line
4041-
* is in fact one physical output line, to prevent corruption of the dump
4042-
* (which could, in the worst case, present an SQL injection vulnerability
4043-
* if someone were to incautiously load a dump containing objects with
4044-
* maliciously crafted names).
4045-
*
4046-
* The result is a freshly malloc'd string. If the input string is NULL,
4047-
* return a malloc'ed empty string, unless want_hyphen, in which case return a
4048-
* malloc'ed hyphen.
4049-
*
4050-
* Note that we currently don't bother to quote names, meaning that the name
4051-
* fields aren't automatically parseable. "pg_restore -L" doesn't care because
4052-
* it only examines the dumpId field, but someday we might want to try harder.
4053-
*/
4054-
static char *
4055-
sanitize_line(const char *str, bool want_hyphen)
4056-
{
4057-
char *result;
4058-
char *s;
4059-
4060-
if (!str)
4061-
return pg_strdup(want_hyphen ? "-" : "");
4062-
4063-
result = pg_strdup(str);
4064-
4065-
for (s = result; *s != '\0'; s++)
4066-
{
4067-
if (*s == '\n' || *s == '\r')
4068-
*s = ' ';
4069-
}
4070-
4071-
return result;
4072-
}
4073-
40744037
/*
40754038
* Write the file header for a custom-format archive
40764039
*/

‎src/bin/pg_dump/pg_dump.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2802,11 +2802,14 @@ dumpTableData(Archive *fout, const TableDataInfo *tdinfo)
28022802
forcePartitionRootLoad(tbinfo)))
28032803
{
28042804
TableInfo *parentTbinfo;
2805+
char *sanitized;
28052806

28062807
parentTbinfo = getRootTableInfo(tbinfo);
28072808
copyFrom = fmtQualifiedDumpable(parentTbinfo);
2809+
sanitized = sanitize_line(copyFrom, true);
28082810
printfPQExpBuffer(copyBuf, "-- load via partition root %s",
2809-
copyFrom);
2811+
sanitized);
2812+
free(sanitized);
28102813
tdDefn = pg_strdup(copyBuf->data);
28112814
}
28122815
else

‎src/bin/pg_dump/pg_dumpall.c

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1492,7 +1492,13 @@ dumpUserConfig(PGconn *conn, const char *username)
14921492
res = executeQuery(conn, buf->data);
14931493

14941494
if (PQntuples(res) > 0)
1495-
fprintf(OPF, "\n--\n-- User Config \"%s\"\n--\n\n", username);
1495+
{
1496+
char *sanitized;
1497+
1498+
sanitized = sanitize_line(username, true);
1499+
fprintf(OPF, "\n--\n-- User Config \"%s\"\n--\n\n", sanitized);
1500+
free(sanitized);
1501+
}
14961502

14971503
for (int i = 0; i < PQntuples(res); i++)
14981504
{
@@ -1594,6 +1600,7 @@ dumpDatabases(PGconn *conn)
15941600
for (i = 0; i < PQntuples(res); i++)
15951601
{
15961602
char *dbname = PQgetvalue(res, i, 0);
1603+
char *sanitized;
15971604
const char *create_opts;
15981605
int ret;
15991606

@@ -1610,7 +1617,9 @@ dumpDatabases(PGconn *conn)
16101617

16111618
pg_log_info("dumping database \"%s\"", dbname);
16121619

1613-
fprintf(OPF, "--\n-- Database \"%s\" dump\n--\n\n", dbname);
1620+
sanitized = sanitize_line(dbname, true);
1621+
fprintf(OPF, "--\n-- Database \"%s\" dump\n--\n\n", sanitized);
1622+
free(sanitized);
16141623

16151624
/*
16161625
* We assume that "template1" and "postgres" already exist in the

‎src/bin/pg_dump/t/002_pg_dump.pl

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2222,6 +2222,27 @@
22222222
},
22232223
},
22242224
2225+
'newline of role or table name in comment' => {
2226+
create_sql => qq{CREATE ROLE regress_newline;
2227+
ALTER ROLE regress_newline SET enable_seqscan = off;
2228+
ALTER ROLE regress_newline
2229+
RENAME TO "regress_newline\nattack";
2230+
2231+
-- meet getPartitioningInfo() "unsafe" condition
2232+
CREATE TYPE pp_colors AS
2233+
ENUM ('green', 'blue', 'black');
2234+
CREATE TABLE pp_enumpart (a pp_colors)
2235+
PARTITION BY HASH (a);
2236+
CREATE TABLE pp_enumpart1 PARTITION OF pp_enumpart
2237+
FOR VALUES WITH (MODULUS 2, REMAINDER 0);
2238+
CREATE TABLE pp_enumpart2 PARTITION OF pp_enumpart
2239+
FOR VALUES WITH (MODULUS 2, REMAINDER 1);
2240+
ALTER TABLE pp_enumpart
2241+
RENAME TO "pp_enumpart\nattack";},
2242+
regexp => qr/\n--[^\n]*\nattack/s,
2243+
like => {},
2244+
},
2245+
22252246
'CREATE TABLESPACE regress_dump_tablespace' => {
22262247
create_order => 2,
22272248
create_sql => q(

‎src/bin/pg_dump/t/003_pg_dump_with_server.pl

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,22 @@
1616
$node->init;
1717
$node->start;
1818

19+
#########################################
20+
# pg_dumpall: newline in database name
21+
22+
$node->safe_psql('postgres', qq{CREATE DATABASE "regress_\nattack"});
23+
24+
my (@cmd, $stdout, $stderr);
25+
@cmd = ("pg_dumpall", '--port' => $port, '--exclude-database=postgres');
26+
print("# Running: " . join(" ", @cmd) . "\n");
27+
my $result = IPC::Run::run \@cmd, '>' => \$stdout, '2>' => \$stderr;
28+
ok(!$result, "newline in dbname: exit code not 0");
29+
like(
30+
$stderr,
31+
qr/shell command argument contains a newline/,
32+
"newline in dbname: stderr matches");
33+
unlike($stdout, qr/^attack/m, "newline in dbname: no comment escape");
34+
1935
#########################################
2036
# Verify that dumping foreign data includes only foreign tables of
2137
# matching servers

0 commit comments

Comments
 (0)