Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 2e5e90d

Browse files
committed
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
1 parent ec3aebd commit 2e5e90d

File tree

1 file changed

+47
-5
lines changed

1 file changed

+47
-5
lines changed

src/bin/pg_dump/pg_dumpall.c

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2129,7 +2129,7 @@ doConnStrQuoting(PQExpBuffer buf, const char *str)
21292129

21302130
/*
21312131
* Append the given string to the shell command being built in the buffer,
2132-
* with suitable shell-style quoting.
2132+
* with suitable shell-style quoting to create exactly one argument.
21332133
*
21342134
* Forbid LF or CR characters, which have scant practical use beyond designing
21352135
* security breaches. The Windows command shell is unusable as a conduit for
@@ -2161,8 +2161,20 @@ doShellQuoting(PQExpBuffer buf, const char *str)
21612161
}
21622162
appendPQExpBufferChar(buf, '\'');
21632163
#else /* WIN32 */
2164+
int backslash_run_length = 0;
21642165

2165-
appendPQExpBufferChar(buf, '"');
2166+
/*
2167+
* A Windows system() argument experiences two layers of interpretation.
2168+
* First, cmd.exe interprets the string. Its behavior is undocumented,
2169+
* but a caret escapes any byte except LF or CR that would otherwise have
2170+
* special meaning. Handling of a caret before LF or CR differs between
2171+
* "cmd.exe /c" and other modes, and it is unusable here.
2172+
*
2173+
* Second, the new process parses its command line to construct argv (see
2174+
* https://msdn.microsoft.com/en-us/library/17w5ykft.aspx). This treats
2175+
* backslash-double quote sequences specially.
2176+
*/
2177+
appendPQExpBufferStr(buf, "^\"");
21662178
for (p = str; *p; p++)
21672179
{
21682180
if (*p == '\n' || *p == '\r')
@@ -2173,11 +2185,41 @@ doShellQuoting(PQExpBuffer buf, const char *str)
21732185
exit(EXIT_FAILURE);
21742186
}
21752187

2188+
/* Change N backslashes before a double quote to 2N+1 backslashes. */
21762189
if (*p == '"')
2177-
appendPQExpBufferStr(buf, "\\\"");
2190+
{
2191+
while (backslash_run_length)
2192+
{
2193+
appendPQExpBufferStr(buf, "^\\");
2194+
backslash_run_length--;
2195+
}
2196+
appendPQExpBufferStr(buf, "^\\");
2197+
}
2198+
else if (*p == '\\')
2199+
backslash_run_length++;
21782200
else
2179-
appendPQExpBufferChar(buf, *p);
2201+
backslash_run_length = 0;
2202+
2203+
/*
2204+
* Decline to caret-escape the most mundane characters, to ease
2205+
* debugging and lest we approach the command length limit.
2206+
*/
2207+
if (!((*p >= 'a' && *p <= 'z') ||
2208+
(*p >= 'A' && *p <= 'Z') ||
2209+
(*p >= '0' && *p <= '9')))
2210+
appendPQExpBufferChar(buf, '^');
2211+
appendPQExpBufferChar(buf, *p);
2212+
}
2213+
2214+
/*
2215+
* Change N backslashes at end of argument to 2N backslashes, because they
2216+
* precede the double quote that terminates the argument.
2217+
*/
2218+
while (backslash_run_length)
2219+
{
2220+
appendPQExpBufferStr(buf, "^\\");
2221+
backslash_run_length--;
21802222
}
2181-
appendPQExpBufferChar(buf, '"');
2223+
appendPQExpBufferStr(buf, "^\"");
21822224
#endif /* WIN32 */
21832225
}

0 commit comments

Comments
 (0)