Reject, in pg_dumpall, names containing CR or LF.
authorNoah Misch <[email protected]>
Mon, 8 Aug 2016 14:07:46 +0000 (10:07 -0400)
committerNoah Misch <[email protected]>
Mon, 8 Aug 2016 14:07:52 +0000 (10:07 -0400)
These characters prematurely terminate Windows shell command processing,
causing the shell to execute a prefix of the intended command.  The
chief alternative to rejecting these characters was to bypass the
Windows shell with CreateProcess(), but the ability to use such names
has little value.  Back-patch to 9.1 (all supported versions).

This change formally revokes support for these characters in database
names and roles names.  Don't document this; the error message is
self-explanatory, and too few users would benefit.  A future major
release may forbid creation of databases and roles so named.  For now,
check only at known weak points in pg_dumpall.  Future commits will,
without notice, reject affected names from other frontend programs.

Also extend the restriction to pg_dumpall --dbname=CONNSTR arguments and
--file arguments.  Unlike the effects on role name arguments and
database names, this does not reflect a broad policy change.  A
migration to CreateProcess() could lift these two restrictions.

Reviewed by Peter Eisentraut.

Security: CVE-2016-5424

src/bin/pg_dump/pg_dumpall.c

index 3b8e53508565cb332f742f2481aac3fbf3bdad95..29278d7ec98403077bb44fa2c46dd966582c0a95 100644 (file)
@@ -2117,6 +2117,12 @@ doConnStrQuoting(PQExpBuffer buf, const char *str)
 /*
  * Append the given string to the shell command being built in the buffer,
  * with suitable shell-style quoting.
+ *
+ * Forbid LF or CR characters, which have scant practical use beyond designing
+ * security breaches.  The Windows command shell is unusable as a conduit for
+ * arguments containing LF or CR characters.  A future major release should
+ * reject those characters in CREATE ROLE and CREATE DATABASE, because use
+ * there eventually leads to errors here.
  */
 static void
 doShellQuoting(PQExpBuffer buf, const char *str)
@@ -2127,6 +2133,14 @@ doShellQuoting(PQExpBuffer buf, const char *str)
        appendPQExpBufferChar(buf, '\'');
        for (p = str; *p; p++)
        {
+               if (*p == '\n' || *p == '\r')
+               {
+                       fprintf(stderr,
+                                       _("shell command argument contains a newline or carriage return: \"%s\"\n"),
+                                       str);
+                       exit(EXIT_FAILURE);
+               }
+
                if (*p == '\'')
                        appendPQExpBuffer(buf, "'\"'\"'");
                else
@@ -2138,6 +2152,14 @@ doShellQuoting(PQExpBuffer buf, const char *str)
        appendPQExpBufferChar(buf, '"');
        for (p = str; *p; p++)
        {
+               if (*p == '\n' || *p == '\r')
+               {
+                       fprintf(stderr,
+                                       _("shell command argument contains a newline or carriage return: \"%s\"\n"),
+                                       str);
+                       exit(EXIT_FAILURE);
+               }
+
                if (*p == '"')
                        appendPQExpBuffer(buf, "\\\"");
                else