# HG changeset patch # User uta # Date 1371561589 -14400 # Node ID 9749983601cfebdd2f54b8623c5f6793748e3bdd # Parent a86f1b82842564fb98a01760d01b567519e29fb9 8016046: (process) Strict validation of input should be security manager case only [win]. Reviewed-by: alanb, ahgross diff -r a86f1b828425 -r 9749983601cf jdk/src/windows/classes/java/lang/ProcessImpl.java --- a/jdk/src/windows/classes/java/lang/ProcessImpl.java Mon Jun 17 20:31:04 2013 -0700 +++ b/jdk/src/windows/classes/java/lang/ProcessImpl.java Tue Jun 18 17:19:49 2013 +0400 @@ -170,7 +170,19 @@ 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 @@ 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 @@ // 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 @@ 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 @@ 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 @@ // 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 @@ { 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 @@ // [.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); } diff -r a86f1b828425 -r 9749983601cf jdk/test/java/lang/Runtime/exec/ExecCommand.java --- a/jdk/test/java/lang/Runtime/exec/ExecCommand.java Mon Jun 17 20:31:04 2013 -0700 +++ b/jdk/test/java/lang/Runtime/exec/ExecCommand.java Tue Jun 18 17:19:49 2013 +0400 @@ -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 @@ 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,15 +92,29 @@ // 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 @@ -99,14 +122,27 @@ // 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 @@ } // 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 @@ } 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 {