With argument type="cmd", the shQuote function uses backslash as an escape character for double quotes instead of the correct escaping method of doubling these characters. Backslash is not an escape character in cmd.exe. As a result, the escaped strings are not safe to use in cmd.exe or the arguments of system2 on Windows. For example > shQuote('"', type="cmd") [1] "\"\\\"\"" while the correct quotation is '""' = "\"\"". This opens a command injection vulnerability. Example: invoking TYPE to print file contents winCat <- function(filename) { shell(paste0("TYPE ", shQuote(filename))) } # Prints the contents of somefile.txt winCat("somefile.txt") # Arbitrary command execution winCat('" & nslookup google.com & REM "') # Correctly escaped version (error) shell('TYPE """ & nslookup google.com & REM """') For some reason, apparently due to the way system and system2 invoke (or do not invoke) the cmd.exe shell, changing 'shell' to 'system' breaks this. But here's another example showing how the bug also affects the system and system2 commands. winFind <- function(string, filename) { system2("FIND", c( shQuote(string, type="cmd"), shQuote(filename, type="cmd"))) } # correctly searches somefile.txt for "some string" winFind("some string", "somefile.txt") # fails with an error winFind('"', "somefile.txt") # Prints all \-containing lines in WindowsUpdate.log winFind('" C:\\Windows\\WindowsUpdate.log "', "somefile.txt") In addition, command injection also works for some command using 'system', for example, if Azure CLI is installed, then the following injects a command argument <- '" & nslookup google.com & REM "' system2("azure", c("help", shQuote(argument, type="cmd")))
I've been unsuccessful at finding any authoritative documentation from Microsoft about the proper way to escape special chars, but this page http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx says that repeating quotes is not the proper way. The cmd interpreter uses ^ as the escape character, and the next level of processing by CommandLineToArgvW uses a backslash as an escape. What reference makes you think that a backslash is not correct?
The problem is that, on Windows, it depends on the invoked command how the arguments should be escaped and there seems to be no consistent method that would be safe for all commands. In R documentation for v. 3.2.2 (for some reason, nothing is mentioned for v. 3.3.0 here https://stat.ethz.ch/R-manual/R-devel/library/base/html/shQuote.html) it is stated that "... this makes shQuote safe for use with file paths in system, and with shell if the default shell is used." and "It will usually be safe to use shQuote to quote arguments of a command, but because system does not use a shell, interpretation of quoted arguments is done by the run-time code of the executable" I would replace 'usually safe' by 'not safe' since the chosen quotation method leaves many (if not most) cases wide open for command injection, as demonstrated in my earlier message, and the problem is not only about using or not using a shell (cmd.exe). To underline my point, here are some more examples demonstrating the complicated and unintuitive escaping rules in cmd.exe. Sometimes one can use backslash to esacpe quotes, sometimes not. The same goes for escaping & as ^&. Here are six valid ways of passing the string 'double "quotes" & dir' to different programs echo double "quotes" ^& dir find "double ""quotes"" & dir" somefile.txt perl -e "print \"double \\\"quotes\\\" ^& dir\"" azure help "double \"quotes\" & dir" azure help "double ""quotes"" & dir" azure help "double """quotes""" & dir" Here are some examples that do not work as supposed to :: prints backslashes and extra quotes echo "double \"quotes\" ^& dir" :: escaping & is not allowed here find "double ""quotes"" ^& dir" somefile.txt :: side effect: attempts to read extra files, e.g., 'double \quotes\' find "double \"quotes\" & dir" somefile.txt find "double """quotes""" & dir" somefile.txt find "double ^"quotes^" ^& dir" somefile.txt and more attempts of trying to pass '" & dir "' to the Azure help command. Only the first one works, all the others result to a command injection. azure help """ & dir """ azure help "\" & dir \"" azure help "\" ^& dir \"" azure help """" & dir """" azure help """" ^& dir """"
You didn't address my question. You claimed that doubling quotes is the right way to escape them in cmd. I don't think that's true. I'm not sure we're doing it right, but I'm not going to make random changes just to get a few examples to work.
You are right, I was too hasty to conclude that doubling quotes would be the correct solution in general. In the (special?) case of the FIND command, it is confirmed as the correct solution here https://technet.microsoft.com/en-us/library/2ca66b22-3b7c-4166-8503-eb75fc53ab46 I have not been able to find any official documentation about quotes in cmd.exe (I don't think it exist), or any complete documentation for that matter. The most accurate resources have been Stack Overflow posts guessing how the interpreter works by experimentation, e.g., http://stackoverflow.com/a/15262019/1426569 http://stackoverflow.com/questions/4094699/how-does-the-windows-command-interpreter-cmd-exe-parse-scripts/4095133#4095133 What you should anyway conclude is that, unlike its apparently bulletproof UNIX counterparts, such as shQuote(x, type="sh"), the current Windows version is NOT, in general, a safe way of "Quoting Strings for Use in OS Shells". I do not know what would be the best way of fixing this but at the very least, the user should be clearly warned about the shortcomings of the implementation.
system() and system2() don't invoke cmd.exe unless you include it in the "command" arg. shell() does by default, though as ?shell says, there are ways to use different shells. As far as I can see, the *only* correct way to quote things on the cmd.exe command line is to precede them with "^". I will add this as an option. So for "internal" commands like TYPE, that's the quoting that's needed. External commands like FIND do their own command line handling, and potentially they all do their own thing. For back compatibility I won't change shQuote(type = "cmd") quoting.
Fixed in R-devel; soon in R-patched. The fix adds a new value for the "type" argument: "cmd2" does escaping suitable for cmd.exe (but probably not for the command it runs).