Protect against symlink attacks

philwebb · philwebb · commit 9b8cb9a4639a · 2018-01-30T12:42:31.000-08:00
Update embedded launch script to no longer change ownership of files or folders that already exist. Fixes spring-projectsgh-11397
diff --git a/spring-boot-integration-tests/spring-boot-launch-script-tests/src/test/java/org/springframework/boot/launchscript/SysVinitLaunchScriptIT.java b/spring-boot-integration-tests/spring-boot-launch-script-tests/src/test/java/org/springframework/boot/launchscript/SysVinitLaunchScriptIT.java
@@ -220,6 +220,25 @@ public void launchWithRelativePidFolder() throws Exception {
 				coloredString(AnsiColor.GREEN, "Stopped [" + extractPid(output) + "]"));
 	}
 
+	@Test
+	public void pidFolderOwnership() throws Exception {
+		String output = doTest("pid-folder-ownership.sh");
+		System.err.println(output);
+		assertThat(output).contains("phil root");
+	}
+
+	@Test
+	public void pidFileOwnership() throws Exception {
+		String output = doTest("pid-file-ownership.sh");
+		assertThat(output).contains("phil root");
+	}
+
+	@Test
+	public void logFileOwnership() throws Exception {
+		String output = doTest("log-file-ownership.sh");
+		assertThat(output).contains("phil root");
+	}
+
 	@Test
 	public void launchWithRelativeLogFolder() throws Exception {
 		String output = doTest("launch-with-relative-log-folder.sh");
diff --git a/spring-boot-integration-tests/spring-boot-launch-script-tests/src/test/resources/scripts/log-file-ownership.sh b/spring-boot-integration-tests/spring-boot-launch-script-tests/src/test/resources/scripts/log-file-ownership.sh
@@ -0,0 +1,20 @@
+source ./test-functions.sh
+install_service
+
+chmod o+w /var/log
+
+useradd phil
+mkdir /phil-files
+chown phil /phil-files
+
+useradd andy
+chown andy /test-service/spring-boot-app.jar
+
+start_service
+stop_service
+
+su - andy -c "ln -s -f /phil-files /var/log/spring-boot-app.log"
+
+start_service
+
+ls -ld /phil-files
diff --git a/spring-boot-integration-tests/spring-boot-launch-script-tests/src/test/resources/scripts/pid-file-ownership.sh b/spring-boot-integration-tests/spring-boot-launch-script-tests/src/test/resources/scripts/pid-file-ownership.sh
@@ -0,0 +1,18 @@
+source ./test-functions.sh
+install_service
+
+useradd phil
+mkdir /phil-files
+chown phil /phil-files
+
+useradd andy
+chown andy /test-service/spring-boot-app.jar
+
+start_service
+stop_service
+
+su - andy -c "ln -s /phil-files /var/run/spring-boot-app/spring-boot-app.pid"
+
+start_service
+
+ls -ld /phil-files
diff --git a/spring-boot-integration-tests/spring-boot-launch-script-tests/src/test/resources/scripts/pid-folder-ownership.sh b/spring-boot-integration-tests/spring-boot-launch-script-tests/src/test/resources/scripts/pid-folder-ownership.sh
@@ -0,0 +1,17 @@
+source ./test-functions.sh
+install_service
+
+chmod o+w /var/run
+
+useradd phil
+mkdir /phil-files
+chown phil /phil-files
+
+useradd andy
+chown andy /test-service/spring-boot-app.jar
+
+su - andy -c "ln -s -f /phil-files /var/run/spring-boot-app"
+
+start_service
+
+ls -ld /phil-files
diff --git a/spring-boot-tools/spring-boot-loader-tools/src/main/resources/org/springframework/boot/loader/tools/launch.script b/spring-boot-tools/spring-boot-loader-tools/src/main/resources/org/springframework/boot/loader/tools/launch.script
@@ -146,12 +146,12 @@ start() {
 do_start() {
   working_dir=$(dirname "$jarfile")
   pushd "$working_dir" > /dev/null
-  mkdir -p "$PID_FOLDER" &> /dev/null
+  if [[ ! -e "$PID_FOLDER" ]]; then
+    mkdir -p "$PID_FOLDER" &> /dev/null
+    chown "$run_user" "$PID_FOLDER"
+  fi
   if [[ -n "$run_user" ]]; then
     checkPermissions || return $?
-    chown "$run_user" "$PID_FOLDER"
-    chown "$run_user" "$pid_file"
-    chown "$run_user" "$log_file"
     if [ $USE_START_STOP_DAEMON = true ] && type start-stop-daemon > /dev/null 2>&1; then
       start-stop-daemon --start --quiet \
         --chuid "$run_user" \
