243 lines
8.5 KiB
Diff
243 lines
8.5 KiB
Diff
From 2179e6005ec9500d5e130f53f7d3ed7c8d4439a9 Mon Sep 17 00:00:00 2001
|
|
From: Noah Misch <noah@leadboat.com>
|
|
Date: Mon, 11 Aug 2025 06:18:59 -0700
|
|
Subject: [PATCH 1/2] 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
|
|
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
|
|
---
|
|
src/bin/pg_dump/dumputils.c | 37 ++++++++++++++++++++
|
|
src/bin/pg_dump/dumputils.h | 1 +
|
|
src/bin/pg_dump/pg_backup_archiver.c | 37 --------------------
|
|
src/bin/pg_dump/pg_dump.c | 5 ++-
|
|
src/bin/pg_dump/pg_dumpall.c | 11 ++++--
|
|
src/bin/pg_dump/t/002_pg_dump.pl | 21 +++++++++++
|
|
src/bin/pg_dump/t/003_pg_dump_with_server.pl | 19 ++++++++--
|
|
7 files changed, 89 insertions(+), 42 deletions(-)
|
|
|
|
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
|
|
index 2de0cef40aa..19ed4842f18 100644
|
|
--- a/src/bin/pg_dump/dumputils.c
|
|
+++ b/src/bin/pg_dump/dumputils.c
|
|
@@ -29,6 +29,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 cb1d98d873e..950cdb5f579 100644
|
|
--- 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 *racls,
|
|
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 1040b1b6b65..3bbd3cb8880 100644
|
|
--- a/src/bin/pg_dump/pg_backup_archiver.c
|
|
+++ b/src/bin/pg_dump/pg_backup_archiver.c
|
|
@@ -73,7 +73,6 @@ static ArchiveHandle *_allocAH(const char *FileSpec, const ArchiveFormat fmt,
|
|
static void _getObjectDescription(PQExpBuffer buf, TocEntry *te,
|
|
ArchiveHandle *AH);
|
|
static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool isData);
|
|
-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);
|
|
@@ -3719,42 +3718,6 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool isData)
|
|
}
|
|
}
|
|
|
|
-/*
|
|
- * 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 29d37158a38..96aac99a944 100644
|
|
--- a/src/bin/pg_dump/pg_dump.c
|
|
+++ b/src/bin/pg_dump/pg_dump.c
|
|
@@ -2396,11 +2396,14 @@ dumpTableData(Archive *fout, 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 41ddc217388..7537922a8f3 100644
|
|
--- a/src/bin/pg_dump/pg_dumpall.c
|
|
+++ b/src/bin/pg_dump/pg_dumpall.c
|
|
@@ -1407,6 +1407,8 @@ dumpUserConfig(PGconn *conn, const char *username)
|
|
if (PQntuples(res) == 1 &&
|
|
!PQgetisnull(res, 0, 0))
|
|
{
|
|
+ char *sanitized;
|
|
+
|
|
/* comment at section start, only if needed */
|
|
if (first)
|
|
{
|
|
@@ -1414,7 +1416,9 @@ dumpUserConfig(PGconn *conn, const char *username)
|
|
first = false;
|
|
}
|
|
|
|
- fprintf(OPF, "--\n-- User Config \"%s\"\n--\n\n", username);
|
|
+ sanitized = sanitize_line(username, true);
|
|
+ fprintf(OPF, "--\n-- User Config \"%s\"\n--\n\n", sanitized);
|
|
+ free(sanitized);
|
|
resetPQExpBuffer(buf);
|
|
makeAlterConfigCommand(conn, PQgetvalue(res, 0, 0),
|
|
"ROLE", username, NULL, NULL,
|
|
@@ -1508,6 +1512,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;
|
|
|
|
@@ -1524,7 +1529,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 93193c0bd4b..cb83714b017 100644
|
|
--- a/src/bin/pg_dump/t/002_pg_dump.pl
|
|
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
|
|
@@ -1430,6 +1430,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 DATABASE regression_invalid...' => {
|
|
create_order => 1,
|
|
create_sql => q(
|
|
--
|
|
2.50.1
|