# HG changeset patch # User uta # Date 1368461360 -14400 # Node ID 80383749fc726e4ff57a1fd99d15d7ab9f0d28c6 # Parent 7c75580b144fc3e7841e1b7fa7e4ae7efb67476e 8012453: (process) Runtime.exec(String) fails if command contains spaces [win] Reviewed-by: alanb diff -r 7c75580b144f -r 80383749fc72 src/share/classes/java/lang/ProcessBuilder.java --- a/src/share/classes/java/lang/ProcessBuilder.java Wed May 01 00:49:21 2013 +0200 +++ b/src/share/classes/java/lang/ProcessBuilder.java Mon May 13 20:09:20 2013 +0400 @@ -29,7 +29,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.io.FileOutputStream; import java.security.AccessControlException; import java.util.Arrays; import java.util.ArrayList; @@ -1025,10 +1024,10 @@ dir, redirects, redirectErrorStream); - } catch (IOException e) { + } catch (IOException | IllegalArgumentException e) { String exceptionInfo = ": " + e.getMessage(); Throwable cause = e; - if (security != null) { + if ((e instanceof IOException) && security != null) { // Can not disclose the fail reason for read-protected files. try { security.checkRead(prog); diff -r 7c75580b144f -r 80383749fc72 src/windows/classes/java/lang/ProcessImpl.java --- a/src/windows/classes/java/lang/ProcessImpl.java Wed May 01 00:49:21 2013 +0200 +++ b/src/windows/classes/java/lang/ProcessImpl.java Mon May 13 20:09:20 2013 +0400 @@ -37,6 +37,9 @@ import java.lang.ProcessBuilder.Redirect; import java.security.AccessController; import java.security.PrivilegedAction; +import java.util.ArrayList; +import java.util.regex.Matcher; +import java.util.regex.Pattern; /* This class is for the exclusive use of ProcessBuilder.start() to * create new processes. @@ -144,6 +147,66 @@ } + private static class LazyPattern { + // Escape-support version: + // "(\")((?:\\\\\\1|.)+?)\\1|([^\\s\"]+)"; + private static final Pattern PATTERN = + Pattern.compile("[^\\s\"]+|\"[^\"]*\""); + }; + + /* Parses the command string parameter into the executable name and + * program arguments. + * + * The command string is broken into tokens. The token separator is a space + * or quota character. The space inside quotation is not a token separator. + * There are no escape sequences. + */ + private static String[] getTokensFromCommand(String command) { + ArrayList matchList = new ArrayList<>(8); + Matcher regexMatcher = LazyPattern.PATTERN.matcher(command); + while (regexMatcher.find()) + matchList.add(regexMatcher.group()); + return matchList.toArray(new String[matchList.size()]); + } + + private static String createCommandLine(boolean isCmdFile, + final String executablePath, + final String cmd[]) + { + StringBuilder cmdbuf = new StringBuilder(80); + + cmdbuf.append(executablePath); + + for (int i = 1; i < cmd.length; ++i) { + cmdbuf.append(' '); + String s = cmd[i]; + if (needsEscaping(isCmdFile, s)) { + cmdbuf.append('"'); + cmdbuf.append(s); + + // The code protects the [java.exe] and console command line + // parser, that interprets the [\"] combination as an escape + // sequence for the ["] char. + // http://msdn.microsoft.com/en-us/library/17w5ykft.aspx + // + // If the argument is an FS path, doubling of the tail [\] + // char is not a problem for non-console applications. + // + // The [\"] sequence is not an escape sequence for the [cmd.exe] + // command line parser. The case of the [""] tail escape + // sequence could not be realized due to the argument validation + // procedure. + if (!isCmdFile && s.endsWith("\\")) { + cmdbuf.append('\\'); + } + cmdbuf.append('"'); + } else { + cmdbuf.append(s); + } + } + 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', '<', '>', '&', '|', '^'}; @@ -226,64 +289,89 @@ } + private boolean isShellFile(String executablePath) { + String upPath = executablePath.toUpperCase(); + return (upPath.endsWith(".CMD") || upPath.endsWith(".BAT")); + } + + private String quoteString(String arg) { + StringBuilder argbuf = new StringBuilder(arg.length() + 2); + return argbuf.append('"').append(arg).append('"').toString(); + } + + private long handle = 0; private OutputStream stdin_stream; private InputStream stdout_stream; private InputStream stderr_stream; - private ProcessImpl(final String cmd[], + private ProcessImpl(String cmd[], final String envblock, final String path, final long[] stdHandles, final boolean redirectErrorStream) throws IOException { - // The [executablePath] is not quoted for any case. - String executablePath = getExecutablePath(cmd[0]); + String cmdstr; + SecurityManager security = System.getSecurityManager(); + boolean allowAmbigousCommands = false; + if (security == null) { + String value = System.getProperty("jdk.lang.Process.allowAmbigousCommands"); + if (value != null) + allowAmbigousCommands = !"false".equalsIgnoreCase(value); + } + if (allowAmbigousCommands) { + // Legacy mode. - // We need to extend the argument verification procedure - // to guarantee the only command file execution for implicit [cmd.exe] - // run. - String upPath = executablePath.toUpperCase(); - boolean isCmdFile = (upPath.endsWith(".CMD") || upPath.endsWith(".BAT")); + // Normalize path if possible. + String executablePath = new File(cmd[0]).getPath(); - StringBuilder cmdbuf = new StringBuilder(80); + // No worry about internal and unpaired ["] . + if (needsEscaping(false, executablePath) ) + executablePath = quoteString(executablePath); - // Quotation protects from interpretation of the [path] argument as - // start of longer path with spaces. Quotation has no influence to - // [.exe] extension heuristic. - cmdbuf.append('"'); - cmdbuf.append(executablePath); - cmdbuf.append('"'); + cmdstr = createCommandLine( + false, //legacy mode doesn't worry about extended verification + executablePath, + cmd); + } else { + String executablePath; + try { + executablePath = getExecutablePath(cmd[0]); + } catch (IllegalArgumentException e) { + // Workaround for the calls like + // Runtime.getRuntime().exec("\"C:\\Program Files\\foo\" bar") - for (int i = 1; i < cmd.length; i++) { - cmdbuf.append(' '); - String s = cmd[i]; - if (needsEscaping(isCmdFile, s)) { - cmdbuf.append('"'); - cmdbuf.append(s); + // No chance to avoid CMD/BAT injection, except to do the work + // right from the beginning. Otherwise we have too many corner + // cases from + // Runtime.getRuntime().exec(String[] cmd [, ...]) + // calls with internal ["] and escape sequences. + + // Restore original command line. + StringBuilder join = new StringBuilder(); + // terminal space in command line is ok + for (String s : cmd) + join.append(s).append(' '); - // The code protects the [java.exe] and console command line - // parser, that interprets the [\"] combination as an escape - // sequence for the ["] char. - // http://msdn.microsoft.com/en-us/library/17w5ykft.aspx - // - // If the argument is an FS path, doubling of the tail [\] - // char is not a problem for non-console applications. - // - // The [\"] sequence is not an escape sequence for the [cmd.exe] - // command line parser. The case of the [""] tail escape - // sequence could not be realized due to the argument validation - // procedure. - if (!isCmdFile && s.endsWith("\\")) { - cmdbuf.append('\\'); - } - cmdbuf.append('"'); - } else { - cmdbuf.append(s); + // Parse the command line again. + cmd = getTokensFromCommand(join.toString()); + executablePath = getExecutablePath(cmd[0]); + + // Check new executable name once more + if (security != null) + security.checkExec(executablePath); } + + // Quotation protects from interpretation of the [path] argument as + // start of longer path with spaces. Quotation has no influence to + // [.exe] extension heuristic. + cmdstr = createCommandLine( + // We need the extended verification procedure for CMD files. + isShellFile(executablePath), + quoteString(executablePath), + cmd); } - String cmdstr = cmdbuf.toString(); handle = create(cmdstr, envblock, path, stdHandles, redirectErrorStream); diff -r 7c75580b144f -r 80383749fc72 test/java/lang/Runtime/exec/ExecCommand.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/java/lang/Runtime/exec/ExecCommand.java Mon May 13 20:09:20 2013 +0400 @@ -0,0 +1,163 @@ +/* + * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + + +/** + * @test + * @bug 8012453 + * @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.FileWriter; +import java.io.IOException; +import java.security.AccessControlException; + +public class ExecCommand { + static class SecurityMan extends SecurityManager { + public static String unquote(String str) + { + int length = (str == null) + ? 0 + : str.length(); + + if (length > 1 + && str.charAt(0) == '\"' + && str.charAt(length - 1) == '\"') + { + return str.substring(1, length - 1); + } + return str; + } + + @Override public void checkExec(String cmd) { + String ncmd = (new File(unquote(cmd))).getPath(); + if ( ncmd.equals(".\\Program") + || ncmd.equals("\".\\Program") + || ncmd.equals(".\\Program Files\\do.cmd") + || ncmd.equals(".\\Program.cmd")) + { + return; + } + super.checkExec(cmd); + } + } + + // Parameters for the Runtime.exec calls + private static final String TEST_RTE_ARG[] = { + ".\\Program Files\\do.cmd", + "\".\\Program Files\\doNot.cmd\" arg", + "\".\\Program Files\\do.cmd\" arg", + // compatibility + "\".\\Program.cmd\" arg", + ".\\Program.cmd arg", + }; + + private static final String doCmdCopy[] = { + ".\\Program.cmd", + ".\\Program Files\\doNot.cmd", + ".\\Program Files\\do.cmd", + }; + + // Golden image for results + private static final String TEST_RTE_GI[][] = { + //Pure system | Legacy mode | Legacy mode & SM + // [.\Program File\do.cmd] + new String[]{"IOException", // [.\Program] not found + "Success", + "IOException"}, //SM - no legacy mode [.\Program] - OK + + // [".\Program File\doNot.cmd" arg] + new String[]{"Success", + "Success", + "AccessControlException"}, //SM - [".\Program] - OK, + // [.\\Program Files\\doNot.cmd] - Fail + + // [".\Program File\do.cmd" arg] + // AccessControlException + new String[]{"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] + }; + + public static void main(String[] _args) throws Exception { + if (!System.getProperty("os.name").startsWith("Windows")) { + return; + } + + // tear up + try { + new File(".\\Program Files").mkdirs(); + for (int i = 0; i < doCmdCopy.length; ++i) { + try (BufferedWriter outCmd = new BufferedWriter( + new FileWriter(doCmdCopy[i]))) { + outCmd.write("@echo %1"); + } + } + } catch (IOException e) { + throw new Error(e.getMessage()); + } + + // action + for (int k = 0; k < 3; ++k) { + switch (k) { + case 1: + System.setProperty("jdk.lang.Process.allowAmbigousCommands", ""); + break; + case 2: + System.setSecurityManager( new SecurityMan() ); + break; + } + for (int i = 0; i < TEST_RTE_ARG.length; ++i) { + String outRes; + try { + Process exec = Runtime.getRuntime().exec(TEST_RTE_ARG[i]); + exec.waitFor(); + outRes = "Success"; + } catch (IOException ioe) { + outRes = "IOException: " + ioe.getMessage(); + } catch (IllegalArgumentException iae) { + outRes = "IllegalArgumentException: " + iae.getMessage(); + } catch (AccessControlException se) { + outRes = "AccessControlException: " + se.getMessage(); + } + + if (!outRes.startsWith(TEST_RTE_GI[i][k])) { + throw new Error("Unexpected output! Step" + k + "" + i + + " \nArgument: " + TEST_RTE_ARG[i] + + "\nExpected: " + TEST_RTE_GI[i][k] + + "\n Output: " + outRes); + } else { + System.out.println("RTE OK:" + TEST_RTE_ARG[i]); + } + } + } + } +}