Use ProcessBuilder to construct program calls

Avoid potential remote code execution through the `parameters`
parameter. They were concatenated to the capture command String
directly, which could lead to the injection of malicious code.

Currently the problem isn't exposed because the potential attacker
can't control the contents of the `parameters` map, since their
contents are chosen from a limited set of values.
This commit is contained in:
Oscar Gonzalez Fernandez 2013-06-05 16:39:55 +02:00
parent 48b29cc6aa
commit 9576e135ad

View file

@ -57,7 +57,7 @@ public class CutyPrint {
private static final Log LOG = LogFactory.getLog(CutyPrint.class); private static final Log LOG = LogFactory.getLog(CutyPrint.class);
private static final String CUTYCAPT_COMMAND = "cutycapt "; private static final String CUTYCAPT_COMMAND = "cutycapt";
// Estimated maximum execution time (ms) // Estimated maximum execution time (ms)
private static final int CUTYCAPT_TIMEOUT = 100000; private static final int CUTYCAPT_TIMEOUT = 100000;
@ -145,9 +145,7 @@ public class CutyPrint {
+ new SimpleDateFormat("yyyyMMddHHmmss").format(new Date()) + new SimpleDateFormat("yyyyMMddHHmmss").format(new Date())
+ extension; + extension;
// Generate capture string String capturePath = CallbackServlet.registerAndCreateURLFor(request,
String captureString = CUTYCAPT_COMMAND;
String url = CallbackServlet.registerAndCreateURLFor(request,
executeOnOriginalContext(new IServletRequestHandler() { executeOnOriginalContext(new IServletRequestHandler() {
@Override @Override
@ -164,34 +162,26 @@ public class CutyPrint {
} }
})); }));
// Add capture destination callback URL ProcessBuilder capture = new ProcessBuilder(CUTYCAPT_COMMAND);
String hostName = resolveLocalHost(request);
captureString += " --url="+ request.getScheme() + "://" + hostName + ":" capture.command().add(
+ request.getLocalPort() + url; " --url=" + createCaptureURL(request, parameters, capturePath));
if (parameters != null) {
captureString += "?";
for (String key : parameters.keySet()) {
captureString += key + "=" + parameters.get(key) + "&";
}
captureString = captureString.substring(0,
(captureString.length() - 1));
}
boolean expanded = Planner boolean expanded = Planner
.guessContainersExpandedByDefaultGivenPrintParameters(parameters); .guessContainersExpandedByDefaultGivenPrintParameters(parameters);
int minWidthForTaskNameColumn = planner int minWidthForTaskNameColumn = planner
.calculateMinimumWidthForTaskNameColumn(expanded); .calculateMinimumWidthForTaskNameColumn(expanded);
int plannerWidth = calculatePlannerWidthForPrintingScreen(planner, int plannerWidth = calculatePlannerWidthForPrintingScreen(planner,
minWidthForTaskNameColumn); minWidthForTaskNameColumn);
captureString += " --min-width=" + plannerWidth; capture.command().add(" --min-width=" + plannerWidth);
int plannerHeight = (expanded ? planner.getAllTasksNumber() : planner int plannerHeight = (expanded ? planner.getAllTasksNumber() : planner
.getTaskNumber()) * TASK_HEIGHT + PRINT_VERTICAL_SPACING; .getTaskNumber()) * TASK_HEIGHT + PRINT_VERTICAL_SPACING;
captureString += " --min-height=" + plannerHeight; capture.command().add(" --min-height=" + plannerHeight);
// Static width and time delay parameters (FIX) // Static width and time delay parameters (FIX)
captureString += " --delay=" + CAPTURE_DELAY; capture.command().add(" --delay=" + CAPTURE_DELAY);
String generatedCSSFile = createCSSFile( String generatedCSSFile = createCSSFile(
absolutePath + "/planner/css/print.css", absolutePath + "/planner/css/print.css",
@ -203,18 +193,18 @@ public class CutyPrint {
minWidthForTaskNameColumn); minWidthForTaskNameColumn);
// Relative user styles // Relative user styles
captureString += " --user-style-path=" + generatedCSSFile; capture.command().add(" --user-style-path=" + generatedCSSFile);
// Destination complete absolute path // Destination complete absolute path
captureString += " --out=" + absolutePath + filename; capture.command().add(" --out=" + absolutePath + filename);
// User language // User language
captureString += " --header=Accept-Language:" capture.command().add(
+ Locales.getCurrent().getLanguage(); " --header=Accept-Language:"
+ Locales.getCurrent().getLanguage());
try { try {
// CutyCapt command execution // CutyCapt command execution
LOG.warn(captureString); LOG.info("calling " + capture.command());
Process printProcess; Process printProcess;
Process serverProcess = null; Process serverProcess = null;
@ -222,21 +212,15 @@ public class CutyPrint {
// If there is a not real X server environment then use Xvfb // If there is a not real X server environment then use Xvfb
if ((System.getenv("DISPLAY") == null) if ((System.getenv("DISPLAY") == null)
|| (System.getenv("DISPLAY").equals(""))) { || (System.getenv("DISPLAY").equals(""))) {
String[] serverEnvironment = { "PATH=$PATH" }; serverProcess = new ProcessBuilder("Xvfb", ":99").start();
serverProcess = Runtime.getRuntime().exec("env - Xvfb :99", capture.environment().put("DISPLAY", ":99.0");
serverEnvironment);
String[] environment = { "DISPLAY=:99.0" };
printProcess = Runtime.getRuntime().exec(captureString,
environment);
} else {
printProcess = Runtime.getRuntime().exec(captureString);
} }
printProcess = capture.start();
try { try {
printProcess.waitFor(); printProcess.waitFor();
printProcess.destroy(); printProcess.destroy();
if ((System.getenv("DISPLAY") == null) if (serverProcess != null) {
|| (System.getenv("DISPLAY").equals(""))) {
serverProcess.destroy(); serverProcess.destroy();
} }
Executions.getCurrent().sendRedirect(filename, "_blank"); Executions.getCurrent().sendRedirect(filename, "_blank");
@ -249,6 +233,23 @@ public class CutyPrint {
} }
} }
private static String createCaptureURL(HttpServletRequest request,
Map<String, String> parameters, String capturePath) {
// Add capture destination callback URL
String hostName = resolveLocalHost(request);
String result = request.getScheme() + "://" + hostName
+ ":" + request.getLocalPort() + capturePath;
if (parameters != null) {
result += "?";
for (String key : parameters.keySet()) {
result += key + "=" + parameters.get(key) + "&";
}
result = result.substring(0,
(result.length() - 1));
}
return result;
}
private static String resolveLocalHost(HttpServletRequest request) { private static String resolveLocalHost(HttpServletRequest request) {
try { try {
InetAddress host = InetAddress.getByName(request.getLocalName()); InetAddress host = InetAddress.getByName(request.getLocalName());