summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNoah Misch2016-08-08 14:07:46 +0000
committerNoah Misch2016-08-08 14:07:53 +0000
commit4837155292f67f10576f3d7204ffd5379bbe3a7b (patch)
tree22e430cf8690d0980d76805874b4f79f63ca23a5
parentffbdab65d3451d72762319829b4bc26dc71a8b48 (diff)
Fix Windows shell argument quoting.
The incorrect quoting may have permitted arbitrary command execution. At a minimum, it gave broader control over the command line to actors supposed to have control over a single argument. Back-patch to 9.1 (all supported versions). Security: CVE-2016-5424
-rw-r--r--src/bin/pg_dump/pg_dumpall.c52
1 files changed, 47 insertions, 5 deletions
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 0d9e135a5b7..47fb7008462 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1956,7 +1956,7 @@ doConnStrQuoting(PQExpBuffer buf, const char *str)
/*
* Append the given string to the shell command being built in the buffer,
- * with suitable shell-style quoting.
+ * with suitable shell-style quoting to create exactly one argument.
*
* Forbid LF or CR characters, which have scant practical use beyond designing
* security breaches. The Windows command shell is unusable as a conduit for
@@ -1988,8 +1988,20 @@ doShellQuoting(PQExpBuffer buf, const char *str)
}
appendPQExpBufferChar(buf, '\'');
#else /* WIN32 */
+ int backslash_run_length = 0;
- appendPQExpBufferChar(buf, '"');
+ /*
+ * A Windows system() argument experiences two layers of interpretation.
+ * First, cmd.exe interprets the string. Its behavior is undocumented,
+ * but a caret escapes any byte except LF or CR that would otherwise have
+ * special meaning. Handling of a caret before LF or CR differs between
+ * "cmd.exe /c" and other modes, and it is unusable here.
+ *
+ * Second, the new process parses its command line to construct argv (see
+ * https://msdn.microsoft.com/en-us/library/17w5ykft.aspx). This treats
+ * backslash-double quote sequences specially.
+ */
+ appendPQExpBufferStr(buf, "^\"");
for (p = str; *p; p++)
{
if (*p == '\n' || *p == '\r')
@@ -2000,11 +2012,41 @@ doShellQuoting(PQExpBuffer buf, const char *str)
exit(EXIT_FAILURE);
}
+ /* Change N backslashes before a double quote to 2N+1 backslashes. */
if (*p == '"')
- appendPQExpBuffer(buf, "\\\"");
+ {
+ while (backslash_run_length)
+ {
+ appendPQExpBufferStr(buf, "^\\");
+ backslash_run_length--;
+ }
+ appendPQExpBufferStr(buf, "^\\");
+ }
+ else if (*p == '\\')
+ backslash_run_length++;
else
- appendPQExpBufferChar(buf, *p);
+ backslash_run_length = 0;
+
+ /*
+ * Decline to caret-escape the most mundane characters, to ease
+ * debugging and lest we approach the command length limit.
+ */
+ if (!((*p >= 'a' && *p <= 'z') ||
+ (*p >= 'A' && *p <= 'Z') ||
+ (*p >= '0' && *p <= '9')))
+ appendPQExpBufferChar(buf, '^');
+ appendPQExpBufferChar(buf, *p);
+ }
+
+ /*
+ * Change N backslashes at end of argument to 2N backslashes, because they
+ * precede the double quote that terminates the argument.
+ */
+ while (backslash_run_length)
+ {
+ appendPQExpBufferStr(buf, "^\\");
+ backslash_run_length--;
}
- appendPQExpBufferChar(buf, '"');
+ appendPQExpBufferStr(buf, "^\"");
#endif /* WIN32 */
}