From 67be67d7864a0be477bc9e5a60e6c2cbad7a874f Mon Sep 17 00:00:00 2001
From: Robert Fink <rf@robertfink.de>
Date: Mon, 24 Oct 2016 22:34:48 -0700
Subject: [PATCH] Check Java binary command for safety (#10)

---
 README.md                                       |  3 ++-
 go-java-launcher/main_test.go                   |  9 +++++++++
 .../launcher-static-bad-java-home.yml           | 10 ++++++++++
 launchlib/launcher.go                           | 17 +++++++++++++++--
 4 files changed, 36 insertions(+), 3 deletions(-)
 create mode 100644 go-java-launcher/test_resources/launcher-static-bad-java-home.yml

diff --git a/README.md b/README.md
index 2e74f695..85f34559 100644
--- a/README.md
+++ b/README.md
@@ -74,7 +74,8 @@ for `CUSTOM_PATH`. The following fixed expansions are supported:
 
 * `{{CWD}}`: The current working directory of the user which executed this process
 
-Expansions are only performed on the values. No expansions are performed on the keys.
+Expansions are only performed on the values. No expansions are performed on the keys. Note that the JAVA_HOME
+environment cannot be overwritten with this mechanism; use the `javaHome` mechanism in `StaticLauncherConfig` instead.
 
 # License
 This repository is made available under the [Apache 2.0 License](http://www.apache.org/licenses/LICENSE-2.0).
diff --git a/go-java-launcher/main_test.go b/go-java-launcher/main_test.go
index 3f09d168..fc70fe85 100644
--- a/go-java-launcher/main_test.go
+++ b/go-java-launcher/main_test.go
@@ -23,6 +23,15 @@ func TestMainMethod(t *testing.T) {
 	LaunchWithConfig("test_resources/launcher-static.yml", "test_resources/launcher-custom.yml")
 }
 
+func TestPanicsWhenJavaHomeIsNotAFile(t *testing.T) {
+	defer func() {
+		if r := recover(); r == nil {
+			t.Errorf("Expected launcher to fail when JAVA_HOME is bad")
+		}
+	}()
+	LaunchWithConfig("test_resources/launcher-static-bad-java-home.yml", "foo")
+}
+
 func TestMainMethodWithoutCustomConfig(t *testing.T) {
 	LaunchWithConfig("test_resources/launcher-static.yml", "foo")
 }
diff --git a/go-java-launcher/test_resources/launcher-static-bad-java-home.yml b/go-java-launcher/test_resources/launcher-static-bad-java-home.yml
new file mode 100644
index 00000000..1fa4491e
--- /dev/null
+++ b/go-java-launcher/test_resources/launcher-static-bad-java-home.yml
@@ -0,0 +1,10 @@
+configType: java
+configVersion: 1
+mainClass: Main
+classpath:
+  - ./test_resources/
+jvmOpts:
+  - '-Xmx4M'
+args:
+  - arg1
+javaHome: /foo/bar
diff --git a/launchlib/launcher.go b/launchlib/launcher.go
index 1dd8d398..69917c19 100644
--- a/launchlib/launcher.go
+++ b/launchlib/launcher.go
@@ -21,13 +21,26 @@ import (
 	"path"
 	"strings"
 	"syscall"
+	"regexp"
 )
 
 const (
-	TemplateDelimsOpen  = "{{"
+	TemplateDelimsOpen = "{{"
 	TemplateDelimsClose = "}}"
+	// Matches characters disallowed in paths we allow to be passed to exec()
+	ExecPathBlackListRegex = `[^\w.\/_\-]`
 )
 
+// Returns true iff the given path is safe to be passed to exec(): must not contain funky characters and be a valid file
+func verifyPathIsSafeForExec(execPath string) string {
+	unsafe, _ := regexp.MatchString(ExecPathBlackListRegex, execPath)
+	_, statError := os.Stat(execPath)
+	if (unsafe || statError != nil) {
+		panic("Failed to determine is path is safe to execute: " + execPath)
+	}
+	return execPath
+}
+
 type processExecutor interface {
 	Exec(executable string, args []string, env []string) error
 }
@@ -78,7 +91,7 @@ func Launch(staticConfig *StaticLauncherConfig, customConfig *CustomLauncherConf
 
 	javaHome := getJavaHome(staticConfig.JavaHome)
 	fmt.Println("Using JAVA_HOME:", javaHome)
-	javaCommand := path.Join(javaHome, "/bin/java")
+	javaCommand := verifyPathIsSafeForExec(path.Join(javaHome, "/bin/java"))
 
 	classpath := joinClasspathEntries(absolutizeClasspathEntries(workingDir, staticConfig.Classpath))
 	fmt.Println("Classpath:", classpath)