8016046: (process) Strict validation of input should be security manager case only [win].
Reviewed-by: alanb, ahgross
--- 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);
}
--- 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 {