Avoid possible command injection
Because we are running cmd.exe here, it is possible to use a malicious URL to execute arbitrary commands on the shell ( e.g. if url=http://my-patch-notes.com/" & del C:\ ) (Triggered by internal security audit and Fortify analysis.)
This commit is contained in:
@@ -9,6 +9,14 @@ import java.util.StringTokenizer;
|
|||||||
|
|
||||||
public class StringUtil {
|
public class StringUtil {
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @return true if the specified string could be a valid URL (contains no illegal characters)
|
||||||
|
*/
|
||||||
|
public static boolean couldBeValidUrl (String url)
|
||||||
|
{
|
||||||
|
return url.matches("[A-Za-z0-9\\-\\._~:/\\?#\\[\\]@!$&'\\(\\)\\*\\+,;=%]+");
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @return true if the string is null or consists only of whitespace, false otherwise.
|
* @return true if the string is null or consists only of whitespace, false otherwise.
|
||||||
*/
|
*/
|
||||||
|
|||||||
@@ -0,0 +1,28 @@
|
|||||||
|
//
|
||||||
|
// Getdown - application installer, patcher and launcher
|
||||||
|
// Copyright (C) 2004-2016 Getdown authors
|
||||||
|
// https://github.com/threerings/getdown/blob/master/LICENSE
|
||||||
|
|
||||||
|
package com.threerings.getdown.util;
|
||||||
|
|
||||||
|
import org.junit.Test;
|
||||||
|
|
||||||
|
import static com.threerings.getdown.util.StringUtil.couldBeValidUrl;
|
||||||
|
import static org.junit.Assert.assertFalse;
|
||||||
|
import static org.junit.Assert.assertTrue;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Tests {@link StringUtil}.
|
||||||
|
*/
|
||||||
|
public class StringUtilTest
|
||||||
|
{
|
||||||
|
@Test public void testCouldBeValidUrl ()
|
||||||
|
{
|
||||||
|
assertTrue(couldBeValidUrl("http://www.foo.com/"));
|
||||||
|
assertTrue(couldBeValidUrl("http://www.foo.com/A-B-C/1_2_3/~bar/q.jsp?x=u+i&y=2;3;4#baz%20baz"));
|
||||||
|
assertTrue(couldBeValidUrl("https://user:secret@www.foo.com/"));
|
||||||
|
|
||||||
|
assertFalse(couldBeValidUrl("http://www.foo.com & echo hello"));
|
||||||
|
assertFalse(couldBeValidUrl("http://www.foo.com\""));
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -36,6 +36,7 @@ import com.samskivert.util.StringUtil;
|
|||||||
import com.threerings.getdown.data.Digest;
|
import com.threerings.getdown.data.Digest;
|
||||||
import com.threerings.getdown.data.SysProps;
|
import com.threerings.getdown.data.SysProps;
|
||||||
import static com.threerings.getdown.Log.log;
|
import static com.threerings.getdown.Log.log;
|
||||||
|
import static com.threerings.getdown.util.StringUtil.couldBeValidUrl;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* The main application entry point for Getdown.
|
* The main application entry point for Getdown.
|
||||||
@@ -194,6 +195,11 @@ public class GetdownApp
|
|||||||
|
|
||||||
@Override
|
@Override
|
||||||
protected void showDocument (String url) {
|
protected void showDocument (String url) {
|
||||||
|
if (!couldBeValidUrl(url)) {
|
||||||
|
// command injection would be possible if we allowed e.g. spaces and double quotes
|
||||||
|
log.warning("Invalid document URL.", "url", url);
|
||||||
|
return;
|
||||||
|
}
|
||||||
String[] cmdarray;
|
String[] cmdarray;
|
||||||
if (RunAnywhere.isWindows()) {
|
if (RunAnywhere.isWindows()) {
|
||||||
String osName = System.getProperty("os.name", "");
|
String osName = System.getProperty("os.name", "");
|
||||||
|
|||||||
Reference in New Issue
Block a user