diff options
author | Noah Misch | 2016-08-08 14:07:46 +0000 |
---|---|---|
committer | Noah Misch | 2016-08-08 14:07:53 +0000 |
commit | 4837155292f67f10576f3d7204ffd5379bbe3a7b (patch) | |
tree | 22e430cf8690d0980d76805874b4f79f63ca23a5 | |
parent | ffbdab65d3451d72762319829b4bc26dc71a8b48 (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.c | 52 |
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 */ } |