]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Convert newlines to spaces in names written in v11+ pg_dump comments.
authorNoah Misch <noah@leadboat.com>
Mon, 11 Aug 2025 13:18:59 +0000 (06:18 -0700)
committerNoah Misch <noah@leadboat.com>
Mon, 11 Aug 2025 13:19:04 +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

src/bin/pg_dump/dumputils.c
src/bin/pg_dump/dumputils.h
src/bin/pg_dump/pg_backup_archiver.c
src/bin/pg_dump/pg_dump.c
src/bin/pg_dump/pg_dumpall.c
src/bin/pg_dump/t/002_pg_dump.pl
src/bin/pg_dump/t/003_pg_dump_with_server.pl

index 60d306e7c3a32c3de1ca0ba2f94be7c1c2e0fd74..6d1148c7916af0ba199c8bc9f1be004fc334ed86 100644 (file)
@@ -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.
  *
index 6e97da7487ef192f75db8671cce6ef87a5ac1bb5..8f6b139217ecd0b1eae2a139c8e652a869aeeb68 100644 (file)
@@ -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,
index 6d4a610d19181ce896c7c5830de1a3e8a6541490..5e9a3ea732cf0924f85cbf7b0296901386dc589a 100644 (file)
@@ -74,7 +74,6 @@ static ArchiveHandle *_allocAH(const char *FileSpec, const ArchiveFormat fmt,
                                                           SetupWorkerPtrType setupWorkerPtr);
 static void _getObjectDescription(PQExpBuffer buf, TocEntry *te);
 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);
@@ -3710,42 +3709,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
  */
index 9a10f45a3f04f73e94fb32d2819473420a87a523..bcee118a0ce0a06b7c43f50e6217caa51f562946 100644 (file)
@@ -2538,11 +2538,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
index 0b8dcde46eadc3dbb74607c44f7a95597c10b8b6..5b286c7d4769d6ef1af52700c015a56f81cabda5 100644 (file)
@@ -1413,6 +1413,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)
                        {
@@ -1420,7 +1422,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,
@@ -1525,6 +1529,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;
 
@@ -1541,7 +1546,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
index da7578709e530d3bd7fe7f19ea3bb564ca8a3149..139c6b393e2cd06f02708f228a4e2ff673976f2e 100644 (file)
@@ -1449,6 +1449,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(
index f9fea9ddcfe7b9df633389f88d0a20f3014d44ce..18177232fa9ad90e50e25e9368788988fc4b9883 100644 (file)
@@ -6,7 +6,7 @@ use warnings;
 
 use PostgresNode;
 use TestLib;
-use Test::More tests => 3;
+use Test::More tests => 6;
 
 my $tempdir       = TestLib::tempdir;
 my $tempdir_short = TestLib::tempdir_short;
@@ -17,6 +17,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
@@ -27,7 +43,6 @@ $node->safe_psql('postgres', "CREATE SERVER s1 FOREIGN DATA WRAPPER dummy");
 $node->safe_psql('postgres', "CREATE SERVER s2 FOREIGN DATA WRAPPER dummy");
 $node->safe_psql('postgres', "CREATE FOREIGN TABLE t0 (a int) SERVER s0");
 $node->safe_psql('postgres', "CREATE FOREIGN TABLE t1 (a int) SERVER s1");
-my ($cmd, $stdout, $stderr, $result);
 
 command_fails_like(
        [ "pg_dump", '-p', $port, '--include-foreign-data=s0', 'postgres' ],