8016046: (process) Strict validation of input should be security manager case only [win].

Reviewed-by: alanb, ahgross

src/windows/classes/java/lang/ProcessImpl.java

@@ -170,7 +170,19 @@ final class ProcessImpl extends Process {
         return matchList.toArray(new String[matchList.size()]);
     }
 
-    private static String createCommandLine(boolean isCmdFile,
+    private static final int VERIFICATION_CMD_BAT = 0;
+    private static final int VERIFICATION_WIN32 = 1;
+    private static final int VERIFICATION_LEGACY = 2;
+    private static final char ESCAPE_VERIFICATION[][] = {
+        // We guarantee the only command file execution for implicit [cmd.exe] run.
+        //    http://technet.microsoft.com/en-us/library/bb490954.aspx
+        {' ', '\t', '<', '>', '&', '|', '^'},
+
+        {' ', '\t', '<', '>'},
+        {' ', '\t'}
+    };
+
+    private static String createCommandLine(int verificationType,
                                      final String executablePath,
                                      final String cmd[])
     {
@@ -181,9 +193,8 @@ final class ProcessImpl extends Process {
         for (int i = 1; i < cmd.length; ++i) {
             cmdbuf.append(' ');
             String s = cmd[i];
-            if (needsEscaping(isCmdFile, s)) {
-                cmdbuf.append('"');
-                cmdbuf.append(s);
+            if (needsEscaping(verificationType, s)) {
+                cmdbuf.append('"').append(s);
 
                 // The code protects the [java.exe] and console command line
                 // parser, that interprets the [\"] combination as an escape
@@ -197,7 +208,7 @@ final class ProcessImpl extends Process {
                 // command line parser. The case of the [""] tail escape
                 // sequence could not be realized due to the argument validation
                 // procedure.
-                if (!isCmdFile && s.endsWith("\\")) {
+                if ((verificationType != VERIFICATION_CMD_BAT) && s.endsWith("\\")) {
                     cmdbuf.append('\\');
                 }
                 cmdbuf.append('"');
@@ -208,11 +219,6 @@ final class ProcessImpl extends Process {
         return cmdbuf.toString();
     }
 
-    // We guarantee the only command file execution for implicit [cmd.exe] run.
-    //    http://technet.microsoft.com/en-us/library/bb490954.aspx
-    private static final char CMD_BAT_ESCAPE[] = {' ', '\t', '<', '>', '&', '|', '^'};
-    private static final char WIN32_EXECUTABLE_ESCAPE[] = {' ', '\t', '<', '>'};
-
     private static boolean isQuoted(boolean noQuotesInside, String arg,
             String errorMessage) {
         int lastPos = arg.length() - 1;
@@ -235,7 +241,7 @@ final class ProcessImpl extends Process {
         return false;
     }
 
-    private static boolean needsEscaping(boolean isCmdFile, String arg) {
+    private static boolean needsEscaping(int verificationType, String arg) {
         // Switch off MS heuristic for internal ["].
         // Please, use the explicit [cmd.exe] call
         // if you need the internal ["].
@@ -243,13 +249,12 @@ final class ProcessImpl extends Process {
 
         // For [.exe] or [.com] file the unpaired/internal ["]
         // in the argument is not a problem.
-        boolean argIsQuoted = isQuoted(isCmdFile, arg,
-            "Argument has embedded quote, use the explicit CMD.EXE call.");
+        boolean argIsQuoted = isQuoted(
+            (verificationType == VERIFICATION_CMD_BAT),
+            arg, "Argument has embedded quote, use the explicit CMD.EXE call.");
 
         if (!argIsQuoted) {
-            char testEscape[] = isCmdFile
-                    ? CMD_BAT_ESCAPE
-                    : WIN32_EXECUTABLE_ESCAPE;
+            char testEscape[] = ESCAPE_VERIFICATION[verificationType];
             for (int i = 0; i < testEscape.length; ++i) {
                 if (arg.indexOf(testEscape[i]) >= 0) {
                     return true;
@@ -315,24 +320,26 @@ final class ProcessImpl extends Process {
     {
         String cmdstr;
         SecurityManager security = System.getSecurityManager();
-        boolean allowAmbigousCommands = false;
+        boolean allowAmbiguousCommands = false;
         if (security == null) {
-            String value = System.getProperty("jdk.lang.Process.allowAmbigousCommands");
+            allowAmbiguousCommands = true;
+            String value = System.getProperty("jdk.lang.Process.allowAmbiguousCommands");
             if (value != null)
-                allowAmbigousCommands = !"false".equalsIgnoreCase(value);
+                allowAmbiguousCommands = !"false".equalsIgnoreCase(value);
         }
-        if (allowAmbigousCommands) {
+        if (allowAmbiguousCommands) {
             // Legacy mode.
 
             // Normalize path if possible.
             String executablePath = new File(cmd[0]).getPath();
 
-            // No worry about internal and unpaired ["] .
-            if (needsEscaping(false, executablePath) )
+            // No worry about internal, unpaired ["], and redirection/piping.
+            if (needsEscaping(VERIFICATION_LEGACY, executablePath) )
                 executablePath = quoteString(executablePath);
 
             cmdstr = createCommandLine(
-                false, //legacy mode doesn't worry about extended verification
+                //legacy mode doesn't worry about extended verification
+                VERIFICATION_LEGACY,
                 executablePath,
                 cmd);
         } else {
@@ -369,7 +376,9 @@ final class ProcessImpl extends Process {
             // [.exe] extension heuristic.
             cmdstr = createCommandLine(
                     // We need the extended verification procedure for CMD files.
-                    isShellFile(executablePath),
+                    isShellFile(executablePath)
+                        ? VERIFICATION_CMD_BAT
+                        : VERIFICATION_WIN32,
                     quoteString(executablePath),
                     cmd);
         }

test/java/lang/Runtime/exec/ExecCommand.java

@@ -24,15 +24,18 @@
 
 /**
  * @test
- * @bug 8012453
+ * @bug 8012453 8016046
  * @run main/othervm ExecCommand
  * @summary workaround for legacy applications with Runtime.getRuntime().exec(String command)
  */
 
 import java.io.BufferedWriter;
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.FileWriter;
 import java.io.IOException;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
 import java.security.AccessControlException;
 
 public class ExecCommand {
@@ -57,16 +60,22 @@ public class ExecCommand {
             if ( ncmd.equals(".\\Program")
               || ncmd.equals("\".\\Program")
               || ncmd.equals(".\\Program Files\\do.cmd")
-              || ncmd.equals(".\\Program.cmd"))
+              || ncmd.equals(".\\Program.cmd")
+              || ncmd.equals("cmd"))
             {
                 return;
             }
             super.checkExec(cmd);
         }
+
+        @Override public void checkDelete(String file) {}
+        @Override public void checkRead(String file) {}
     }
 
     // Parameters for the Runtime.exec calls
     private static final String TEST_RTE_ARG[] = {
+        "cmd /C dir > dirOut.txt",
+        "cmd /C dir > \".\\Program Files\\dirOut.txt\"",
         ".\\Program Files\\do.cmd",
         "\".\\Program Files\\doNot.cmd\" arg",
         "\".\\Program Files\\do.cmd\" arg",
@@ -83,14 +92,28 @@ public class ExecCommand {
 
     // Golden image for results
     private static final String TEST_RTE_GI[][] = {
-                    //Pure system | Legacy mode | Legacy mode & SM
+                    //Def Legacy mode, Enforced mode, Set Legacy mode, Set Legacy mode & SM
+        // [cmd /C dir > dirOut.txt]
+        new String[]{"Success",
+                     "IOException",  // [cmd /C dir ">" dirOut.txt] no redirection
+                     "Success",
+                     "IOException"}, //SM - no legacy mode, bad command
+
+        // [cmd /C dir > ".\Program Files\dirOut.txt"]
+        new String[]{"Success",
+                     "IOException",  // [cmd /C dir ">" ".\Program Files\dirOut.txt"] no redirection
+                     "Success",
+                     "IOException"}, //SM - no legacy mode, bad command
+
         // [.\Program File\do.cmd]
-        new String[]{"IOException",  // [.\Program] not found
+        new String[]{"Success",
+                     "IOException",  // [.\Program] not found
                      "Success",
                      "IOException"}, //SM - no legacy mode [.\Program] - OK
 
         // [".\Program File\doNot.cmd" arg]
         new String[]{"Success",
+                     "Success",
                      "Success",
                      "AccessControlException"}, //SM   - [".\Program] - OK,
                                  //     [.\\Program Files\\doNot.cmd] - Fail
@@ -98,15 +121,28 @@ public class ExecCommand {
         // [".\Program File\do.cmd" arg]
         // AccessControlException
         new String[]{"Success",
+                     "Success",
                      "Success",
                      "Success"}, //SM - [".\Program] - OK,
                                  //     [.\\Program Files\\do.cmd] - OK
 
         // compatibility
-        new String[]{"Success", "Success", "Success"}, //[".\Program.cmd"]
-        new String[]{"Success", "Success", "Success"}  //[.\Program.cmd]
+        new String[]{"Success", "Success", "Success", "Success"}, //[".\Program.cmd"]
+        new String[]{"Success", "Success", "Success", "Success"}  //[.\Program.cmd]
     };
 
+    private static void deleteOut(String path) {
+        try {
+            Files.delete(FileSystems.getDefault().getPath(path));
+        } catch (IOException ex) {
+            //that is OK
+        }
+    }
+    private static void checkOut(String path) throws FileNotFoundException {
+        if (Files.notExists(FileSystems.getDefault().getPath(path)))
+            throw new FileNotFoundException(path);
+    }
+
     public static void main(String[] _args) throws Exception {
         if (!System.getProperty("os.name").startsWith("Windows")) {
             return;
@@ -126,20 +162,51 @@ public class ExecCommand {
         }
 
         // action
-        for (int k = 0; k < 3; ++k) {
+        for (int k = 0; k < 4; ++k) {
             switch (k) {
+            case 0:
+                // the "jdk.lang.Process.allowAmbiguousCommands" is undefined
+                // "true" by default with the legacy verification procedure
+                break;
             case 1:
-                System.setProperty("jdk.lang.Process.allowAmbigousCommands", "");
+                System.setProperty("jdk.lang.Process.allowAmbiguousCommands", "false");
                 break;
             case 2:
+                System.setProperty("jdk.lang.Process.allowAmbiguousCommands", "");
+                break;
+            case 3:
                 System.setSecurityManager( new SecurityMan() );
                 break;
             }
             for (int i = 0; i < TEST_RTE_ARG.length; ++i) {
                 String outRes;
                 try {
+                    // tear up
+                    switch (i) {
+                    case 0:
+                        // [cmd /C dir > dirOut.txt]
+                        deleteOut(".\\dirOut.txt");
+                        break;
+                    case 1:
+                        // [cmd /C dir > ".\Program Files\dirOut.txt"]
+                        deleteOut(".\\Program Files\\dirOut.txt");
+                        break;
+                    }
+
                     Process exec = Runtime.getRuntime().exec(TEST_RTE_ARG[i]);
                     exec.waitFor();
+
+                    //exteded check
+                    switch (i) {
+                    case 0:
+                        // [cmd /C dir > dirOut.txt]
+                        checkOut(".\\dirOut.txt");
+                        break;
+                    case 1:
+                        // [cmd /C dir > ".\Program Files\dirOut.txt"]
+                        checkOut(".\\Program Files\\dirOut.txt");
+                        break;
+                    }
                     outRes = "Success";
                 } catch (IOException ioe) {
                     outRes = "IOException: " + ioe.getMessage();
@@ -150,8 +217,8 @@ public class ExecCommand {
                 }
 
                 if (!outRes.startsWith(TEST_RTE_GI[i][k])) {
-                    throw new Error("Unexpected output! Step" + k + "" + i
-                                + " \nArgument: " + TEST_RTE_ARG[i]
+                    throw new Error("Unexpected output! Step" + k + ":" + i
+                                + "\nArgument: " + TEST_RTE_ARG[i]
                                 + "\nExpected: " + TEST_RTE_GI[i][k]
                                 + "\n  Output: " + outRes);
                 } else {