git.postgresql.org Git - postgresql.git/commitdiff

git projects / postgresql.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 605fdb9)
Convert newlines to spaces in names written in v11+ pg_dump comments.
2025年8月11日 13:18:59 +0000 (06:18 -0700)
2025年8月11日 13:19:03 +0000 (06:19 -0700)
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
bc8cd50fefd369b217f80078585c486505aafb62 (back-patched to v11+ in
2023-05 releases) introduced the pg_dump case. Commit
6cbdbd9e8d8f2986fde44f2431ed8d0c8fce7f5d (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


diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index 73ce34346b2784b19a6b7188797d68175fc79a48..4c02a9863c471d7bf039f631d6182133982540d3 100644 (file)
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -31,6 +31,43 @@ static void AddAcl(PQExpBuffer aclbuf, const char *keyword,
const char *subname);
+/*
+ * Sanitize a string to be included in an SQL comment or TOC listing, by
+ * replacing any newlines with spaces. This ensures each logical output line
+ * is in fact one physical output line, to prevent corruption of the dump
+ * (which could, in the worst case, present an SQL injection vulnerability
+ * if someone were to incautiously load a dump containing objects with
+ * maliciously crafted names).
+ *
+ * The result is a freshly malloc'd string. If the input string is NULL,
+ * return a malloc'ed empty string, unless want_hyphen, in which case return a
+ * malloc'ed hyphen.
+ *
+ * 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.
+ */
+char *
+sanitize_line(const char *str, bool want_hyphen)
+{
+ char *result;
+ char *s;
+
+ if (!str)
+ return pg_strdup(want_hyphen ? "-" : "");
+
+ result = pg_strdup(str);
+
+ for (s = result; *s != '0円'; s++)
+ {
+ if (*s == '\n' || *s == '\r')
+ *s = ' ';
+ }
+
+ return result;
+}
+
+
/*
* Build GRANT/REVOKE command(s) for an object.
*
diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h
index 91c6e612e282ef3874e20c94a45686f8514b95b4..71dd3f12a8dd109f9b0f724a5dd4f39aeb8b2b12 100644 (file)
--- a/src/bin/pg_dump/dumputils.h
+++ b/src/bin/pg_dump/dumputils.h
@@ -36,6 +36,7 @@
#endif
+extern char *sanitize_line(const char *str, bool want_hyphen);
extern bool buildACLCommands(const char *name, const char *subname, const char *nspname,
const char *type, const char *acls, const char *baseacls,
const char *owner, const char *prefix, int remoteVersion,
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index e0b9ea0259f0d230b16115a05e5f0cbbe7caf349..a13e221caf40a79c145b5e05025ab44d948aa614 100644 (file)
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -57,7 +57,6 @@ static ArchiveHandle *_allocAH(const char *FileSpec, const ArchiveFormat fmt,
DataDirSyncMethod sync_method);
static void _getObjectDescription(PQExpBuffer buf, const TocEntry *te);
static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, const char *pfx);
-static char *sanitize_line(const char *str, bool want_hyphen);
static void _doSetFixedOutputState(ArchiveHandle *AH);
static void _doSetSessionAuth(ArchiveHandle *AH, const char *user);
static void _reconnectToDB(ArchiveHandle *AH, const char *dbname);
@@ -4035,42 +4034,6 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, const char *pfx)
}
}
-/*
- * Sanitize a string to be included in an SQL comment or TOC listing, by
- * replacing any newlines with spaces. This ensures each logical output line
- * is in fact one physical output line, to prevent corruption of the dump
- * (which could, in the worst case, present an SQL injection vulnerability
- * if someone were to incautiously load a dump containing objects with
- * maliciously crafted names).
- *
- * The result is a freshly malloc'd string. If the input string is NULL,
- * return a malloc'ed empty string, unless want_hyphen, in which case return a
- * malloc'ed hyphen.
- *
- * 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.
- */
-static char *
-sanitize_line(const char *str, bool want_hyphen)
-{
- char *result;
- char *s;
-
- if (!str)
- return pg_strdup(want_hyphen ? "-" : "");
-
- result = pg_strdup(str);
-
- for (s = result; *s != '0円'; s++)
- {
- if (*s == '\n' || *s == '\r')
- *s = ' ';
- }
-
- return result;
-}
-
/*
* Write the file header for a custom-format archive
*/
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index f867a1c0cbe63e6b73fbfe81274df483aee08df9..3e5729ad7fb6975bb60bba070696f162e36f870f 100644 (file)
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -2802,11 +2802,14 @@ dumpTableData(Archive *fout, const TableDataInfo *tdinfo)
forcePartitionRootLoad(tbinfo)))
{
TableInfo *parentTbinfo;
+ char *sanitized;
parentTbinfo = getRootTableInfo(tbinfo);
copyFrom = fmtQualifiedDumpable(parentTbinfo);
+ sanitized = sanitize_line(copyFrom, true);
printfPQExpBuffer(copyBuf, "-- load via partition root %s",
- copyFrom);
+ sanitized);
+ free(sanitized);
tdDefn = pg_strdup(copyBuf->data);
}
else
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 27aa1b656989c2cd0f40fd4c7d324282969eec91..3e7f930c9402a0b5de7fabbe6799893780251702 100644 (file)
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1492,7 +1492,13 @@ dumpUserConfig(PGconn *conn, const char *username)
res = executeQuery(conn, buf->data);
if (PQntuples(res) > 0)
- fprintf(OPF, "\n--\n-- User Config \"%s\"\n--\n\n", username);
+ {
+ char *sanitized;
+
+ sanitized = sanitize_line(username, true);
+ fprintf(OPF, "\n--\n-- User Config \"%s\"\n--\n\n", sanitized);
+ free(sanitized);
+ }
for (int i = 0; i < PQntuples(res); i++)
{
@@ -1594,6 +1600,7 @@ dumpDatabases(PGconn *conn)
for (i = 0; i < PQntuples(res); i++)
{
char *dbname = PQgetvalue(res, i, 0);
+ char *sanitized;
const char *create_opts;
int ret;
@@ -1610,7 +1617,9 @@ dumpDatabases(PGconn *conn)
pg_log_info("dumping database \"%s\"", dbname);
- fprintf(OPF, "--\n-- Database \"%s\" dump\n--\n\n", dbname);
+ sanitized = sanitize_line(dbname, true);
+ fprintf(OPF, "--\n-- Database \"%s\" dump\n--\n\n", sanitized);
+ free(sanitized);
/*
* We assume that "template1" and "postgres" already exist in the
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 1aea8a25d5e7e12b09908e5f6238745678a77d35..d3a9d4db47ef56e82313fab16d3106bed6c6bf6a 100644 (file)
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -2222,6 +2222,27 @@ my %tests = (
},
},
+ 'newline of role or table name in comment' => {
+ create_sql => qq{CREATE ROLE regress_newline;
+ ALTER ROLE regress_newline SET enable_seqscan = off;
+ ALTER ROLE regress_newline
+ RENAME TO "regress_newline\nattack";
+
+ -- meet getPartitioningInfo() "unsafe" condition
+ CREATE TYPE pp_colors AS
+ ENUM ('green', 'blue', 'black');
+ CREATE TABLE pp_enumpart (a pp_colors)
+ PARTITION BY HASH (a);
+ CREATE TABLE pp_enumpart1 PARTITION OF pp_enumpart
+ FOR VALUES WITH (MODULUS 2, REMAINDER 0);
+ CREATE TABLE pp_enumpart2 PARTITION OF pp_enumpart
+ FOR VALUES WITH (MODULUS 2, REMAINDER 1);
+ ALTER TABLE pp_enumpart
+ RENAME TO "pp_enumpart\nattack";},
+ regexp => qr/\n--[^\n]*\nattack/s,
+ like => {},
+ },
+
'CREATE TABLESPACE regress_dump_tablespace' => {
create_order => 2,
create_sql => q(
diff --git a/src/bin/pg_dump/t/003_pg_dump_with_server.pl b/src/bin/pg_dump/t/003_pg_dump_with_server.pl
index 8dc014ed6ed34beb648ea60c9f3f39c9aa1b1a3e..c83d3899dfbf7948c91d82b5e39584f4194db34a 100644 (file)
--- a/src/bin/pg_dump/t/003_pg_dump_with_server.pl
+++ b/src/bin/pg_dump/t/003_pg_dump_with_server.pl
@@ -16,6 +16,22 @@ my $port = $node->port;
$node->init;
$node->start;
+#########################################
+# pg_dumpall: newline in database name
+
+$node->safe_psql('postgres', qq{CREATE DATABASE "regress_\nattack"});
+
+my (@cmd, $stdout, $stderr);
+@cmd = ("pg_dumpall", '--port' => $port, '--exclude-database=postgres');
+print("# Running: " . join(" ", @cmd) . "\n");
+my $result = IPC::Run::run \@cmd, '>' => \$stdout, '2>' => \$stderr;
+ok(!$result, "newline in dbname: exit code not 0");
+like(
+ $stderr,
+ qr/shell command argument contains a newline/,
+ "newline in dbname: stderr matches");
+unlike($stdout, qr/^attack/m, "newline in dbname: no comment escape");
+
#########################################
# Verify that dumping foreign data includes only foreign tables of
# matching servers
This is the main PostgreSQL git repository.
RSS Atom

AltStyle によって変換されたページ (->オリジナル) /