Merge pull request #136 from sdgx/patch-notes-url-command-injection

Avoid possible command injection (patch notes URL)

core/src/main/java/com/threerings/getdown/util/StringUtil.java

@@ -9,6 +9,14 @@ import java.util.StringTokenizer;
 
 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.
      */

core/src/test/java/com/threerings/getdown/util/StringUtilTest.java

@@ -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\""));
+    }
+}

launcher/src/main/java/com/threerings/getdown/launcher/GetdownApp.java

@@ -36,6 +36,7 @@ import com.samskivert.util.StringUtil;
 import com.threerings.getdown.data.Digest;
 import com.threerings.getdown.data.SysProps;
 import static com.threerings.getdown.Log.log;
+import static com.threerings.getdown.util.StringUtil.couldBeValidUrl;
 
 /**
  * The main application entry point for Getdown.
@@ -194,6 +195,11 @@ public class GetdownApp
 
             @Override
             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;
                 if (RunAnywhere.isWindows()) {
                     String osName = System.getProperty("os.name", "");