Hi,

as far as I know, on Windows every program can obtain the lpCommandLine argument, used in the call of CreateProcess() from its parent, by calling GetCommandLine() and parse that string as it sees fit. This is in stark contrast with how Unix-like systems create and execute programs, where the system call execve(2) accepts an array of arguments, not a single command line.

There are no fixed rules on how to parse the command line, as witnessed by the different strategies implemented in the C/C++ runtime (which splits the command line according to the rules outlined in [1] to populate the argv[] array in main()), the cmd.exe shell, the wscript.exe runtime, etc.

Consequently, there are no fixed rules on how to encode a command line (specifically, the lpCommandLine argument to CreateProcess()) because it really is up to the invoked program to parse it, whether explicitly or implicitly, according to its own, directly or indirectly implemented rules. Even a C/C++ console program could ignore the result that the runtime automatically provides in argv[] and parse the command line directly, as obtained by GetCommandLine().

Without knowing the parsing rules of the target program, it is not possible to encode a command line correctly for CreateProcess(). I doubt there's a "common denominator" which would cover most cases encountered in practice.

The best we can hope is to implement encoders (and decoders) for specific, widely used runtimes. (BTW, for the C/C++ runtime I prepared an implementation mentioned in [2].)


Greetings
Raffaello

----

[1] https://docs.microsoft.com/en-us/cpp/c-language/parsing-c-command-line-arguments [2] https://mail.openjdk.java.net/pipermail/core-libs-dev/2022-February/086105.html



On 2022-02-24 20:18, Olga Mikhaltsova wrote:
On Fri, 18 Feb 2022 17:21:48 GMT, Olga Mikhaltsova <omikhaltc...@openjdk.org> 
wrote:

This fix made equal processing of strings such as ""C:\\Program Files\\Git\\"" 
before and after JDK-8250568.

For example, it's needed to execute the following command on Windows:
`C:\Windows\SysWOW64\WScript.exe "MyVB.vbs" "C:\Program Files\Git" "Test"`
it's equal to:
`new ProcessBuilder("C:\\Windows\\SysWOW64\\WScript.exe", "MyVB.vbs", ""C:\\Program 
Files\\Git\\"", "Test").start();`

While processing, the 3rd argument ""C:\\Program Files\\Git\\"" treated as 
unquoted due to the condition added in JDK-8250568.

     private static String unQuote(String str) {
     ..
        if (str.endsWith("\\"")) {
             return str;    // not properly quoted, treat as unquoted
         }
     ..
     }


that leads to the additional surrounding by quotes in 
ProcessImpl::createCommandLine(..) because needsEscaping(..) returns true due 
to the space inside the string argument.
As a result the native function CreateProcessW 
(src/java.base/windows/native/libjava/ProcessImpl_md.c) gets the incorrectly 
quoted argument:

pcmd = C:\Windows\SysWOW64\WScript.exe MyVB.vbs ""C:\Program Files\Git"" Test
(jdk.lang.Process.allowAmbiguousCommands = true)
pcmd = "C:\Windows\SysWOW64\WScript.exe" MyVB.vbs ""C:\Program Files\Git\\"" 
Test
(jdk.lang.Process.allowAmbiguousCommands = false)


Obviously, a string ending with `"\\""` must not be started with `"""` to treat 
as unquoted overwise it’s should be treated as properly quoted.

Olga Mikhaltsova has updated the pull request incrementally with one additional 
commit since the last revision:

   Add a new line to the end of test file for JDK-8282008

Roger, writing a test via echo was not a good idea obviously for this particular case 
because of the fact well shown in the doc "4. Everyone Parses Differently", 
https://daviddeley.com/autohotkey/parameters/parameters.htm#WINCRULESMSEX. The task is 
more complicated than it seems at the first glance. The same command line correctly 
parsed in an app written in C/C++ might be incorrectly parsed in a VBS app etc.

The suggestion not to use the path-argument surroundings with `'"'` doesn't fix the issue 
in case of VBS. It leads to a resulting path-argument ending with the doubled backslash in a 
VBS app according to the rules "10.1 The WSH Command Line Parameter Parsing Rules", 
https://daviddeley.com/autohotkey/parameters/parameters.htm#WSH.

Below there are some experiments with an app attached to JDK-8282008:

NO FIX

1. new ProcessBuilder("C:\\Windows\\SysWOW64\\WScript.exe", "MyVB.vbs", ""C:\\Program 
Files\\Git\\"", "Test").start();

1.1 allowAmbiguousCommands = false
    arg[0] = \C:\Program
    arg[1] = Files\Git1\\\
CreateProcessW: pcmd = "C:\Windows\SysWOW64\WScript.exe" MyVB.vbs ""C:\Program 
Files\Git1\\"" Test

1.2 allowAmbiguousCommands = true
    arg[0] = C:\Program
    arg[1] = Files\Git1\
CreateProcessW: pcmd = C:\Windows\SysWOW64\WScript.exe MyVB.vbs ""C:\Program 
Files\Git1"" Test

2. new ProcessBuilder("C:\\Windows\\SysWOW64\\WScript.exe", "MyVB.vbs", "C:\\Program 
Files\\Git\", "Test").start();

2.1 allowAmbiguousCommands = false
    arg[0] = C:\Program Files\Git1\\
    arg[1] = Test
CreateProcessW: pcmd = "C:\Windows\SysWOW64\WScript.exe" MyVB.vbs "C:\Program 
Files\Git1\" Test

2.2 allowAmbiguousCommands = true
    arg[0] = C:\Program Files\Git1\\
    arg[1] = Test
CreateProcessW: pcmd = C:\Windows\SysWOW64\WScript.exe MyVB.vbs "C:\Program 
Files\Git1\" Test


FIXED (as in pr)

1. new ProcessBuilder("C:\\Windows\\SysWOW64\\WScript.exe", "MyVB.vbs", ""C:\\Program 
Files\\Git\\"", "Test").start();

1.1 allowAmbiguousCommands = false
    arg[0] = C:\Program Files\Git1\
    arg[1] = Test
CreateProcessW: pcmd = "C:\Windows\SysWOW64\WScript.exe" MyVB.vbs "C:\Program 
Files\Git1" Test

1.2 allowAmbiguousCommands = true
  arg[0] = C:\Program Files\Git1\
  arg[1] = Test
CreateProcessW: pcmd = C:\Windows\SysWOW64\WScript.exe MyVB.vbs "C:\Program 
Files\Git1" Test
```

Reading the code of unQuote() in terms of logic: "no beginning or ending quote, or too short => not 
quoted", - and it seems that if all these conditions are just opposite (starting and ending quotes and long 
enough) then a string should be treated as quoted but it's not. One exception was added and it's strange that 
it's applied even in case of paired quotes. Is it truly necessary for the security fix to skip (=to treat as 
unquoted) a string starting and ending with `'"'` in case of a `"\\""` tail?

This proposed fix returns back possibility (as was previously) to use surrounding 
with `'"'` as an argument mark-up that allows to pass correctly a path with a 
space inside in case of VBS. Roger, would you be so kind to take a look at this 
small fix once again, please, and to pay attention to VBS parsing arguments problem?!

-------------

PR: https://git.openjdk.java.net/jdk/pull/7504

Reply via email to