fix: restrict app names to prevent command injection

The previous app name validation regex permitted shell metacharacters such as `;`, `$`, backticks, `|`, and `&`. These names were embedded unquoted into the generated git pre-receive hook script, allowing an authenticated user to execute arbitrary commands as the dokku user simply by pushing to a remote with a crafted app name. App names are now restricted to lowercase alphanumerics, dots, and hyphens, and the hook script also quotes the app variable as a defense-in-depth measure.

plugins/common/common.go

@@ -923,12 +923,12 @@ func IsValidAppName(appName string) error {
 		return errors.New("Please specify an app to run the command on")
 	}
 
-	r, _ := regexp.Compile("^[a-z0-9][^/:_A-Z]*$")
+	r, _ := regexp.Compile("^[a-z0-9][a-z0-9.-]*$")
 	if r.MatchString(appName) {
 		return nil
 	}
 
-	return errors.New("App name must begin with lowercase alphanumeric character, and cannot include uppercase characters, colons, or underscores")
+	return errors.New("App name must begin with lowercase alphanumeric character, and may only contain lowercase alphanumerics, dots, and hyphens")
 }
 
 // isValidAppNameOld verifies that the app name matches the old naming restrictions
@@ -937,12 +937,12 @@ func isValidAppNameOld(appName string) error {
 		return errors.New("Please specify an app to run the command on")
 	}
 
-	r, _ := regexp.Compile("^[a-z0-9][^/:A-Z]*$")
+	r, _ := regexp.Compile("^[a-z0-9][a-z0-9.-]*$")
 	if r.MatchString(appName) {
 		return nil
 	}
 
-	return errors.New("App name must begin with lowercase alphanumeric character, and cannot include uppercase characters, or colons")
+	return errors.New("App name must begin with lowercase alphanumeric character, and may only contain lowercase alphanumerics, dots, and hyphens")
 }
 
 // AppDoesNotExist wraps error to include the app name

plugins/common/common_test.go

@@ -77,6 +77,60 @@ func TestCommonVerifyAppNameInvalid(t *testing.T) {
 	Expect(VerifyAppName("1994testApp")).To(HaveOccurred())
 }
 
+func TestCommonIsValidAppName(t *testing.T) {
+	RegisterTestingT(t)
+	validNames := []string{
+		"myapp",
+		"my-app",
+		"my.app",
+		"app123",
+		"123app",
+		"my-app.prod",
+		"a",
+		"0",
+	}
+	for _, name := range validNames {
+		Expect(IsValidAppName(name)).To(Succeed(), "expected %q to be a valid app name", name)
+	}
+}
+
+func TestCommonIsValidAppNameRejectsShellMetacharacters(t *testing.T) {
+	RegisterTestingT(t)
+	invalidNames := []string{
+		"app;id",
+		"app$cmd",
+		"app`whoami`",
+		"app|cat",
+		"app&background",
+		"app>file",
+		"app<file",
+		"app(cmd)",
+		"app{cmd}",
+		"app[cmd]",
+		"app*",
+		"app?",
+		"app!",
+		"app#",
+		"app\\cmd",
+		"app\"quote",
+		"app'quote",
+		"app cmd",
+		"app\tcmd",
+		"app\ncmd",
+		"app~cmd",
+		"app/cmd",
+		"app:cmd",
+		"app_name",
+		"App",
+		"-app",
+		".app",
+		"",
+	}
+	for _, name := range invalidNames {
+		Expect(IsValidAppName(name)).To(HaveOccurred(), "expected %q to be rejected", name)
+	}
+}
+
 func TestCommonVerifyAppName(t *testing.T) {
 	RegisterTestingT(t)
 	Expect(setupTests()).To(Succeed())

plugins/common/functions

@@ -222,10 +222,8 @@ fn-is-valid-app-name() {
   declare desc="verify that the app name matches naming restrictions"
   local APP="$1"
   [[ -z "$APP" ]] && dokku_log_fail "Please specify an app to run the command on"
-  if [[ "$APP" =~ ^[a-z].* ]] || [[ "$APP" =~ ^[0-9].* ]]; then
-    if [[ ! $APP =~ [A-Z] ]] && [[ ! $APP =~ [:] ]] && [[ ! $APP =~ [_] ]]; then
-      return 0
-    fi
+  if [[ "$APP" =~ ^[a-z0-9][a-z0-9.-]*$ ]]; then
+    return 0
   fi
 
   return 1
@@ -235,10 +233,8 @@ fn-is-valid-app-name-old() {
   declare desc="verify that the app name matches the old naming restrictions"
   local APP="$1"
   [[ -z "$APP" ]] && dokku_log_fail "Please specify an app to run the command on"
-  if [[ "$APP" =~ ^[a-z].* ]] || [[ "$APP" =~ ^[0-9].* ]]; then
-    if [[ ! $APP =~ [A-Z] ]] && [[ ! $APP =~ [:] ]]; then
-      return 0
-    fi
+  if [[ "$APP" =~ ^[a-z0-9][a-z0-9.-]*$ ]]; then
+    return 0
   fi
 
   return 1
@@ -265,7 +261,7 @@ is_valid_app_name() {
     return
   fi
 
-  dokku_log_fail "App name must begin with lowercase alphanumeric character, and cannot include uppercase characters, colons, or underscores"
+  dokku_log_fail "App name must begin with lowercase alphanumeric character, and may only contain lowercase alphanumerics, dots, and hyphens"
 }
 
 is_valid_app_name_old() {
@@ -276,7 +272,7 @@ is_valid_app_name_old() {
     return
   fi
 
-  dokku_log_fail "App name must begin with lowercase alphanumeric character, and cannot include uppercase characters, or colons"
+  dokku_log_fail "App name must begin with lowercase alphanumeric character, and may only contain lowercase alphanumerics, dots, and hyphens"
 }
 
 verify_app_name() {

plugins/git/internal-functions

@@ -464,7 +464,7 @@ fn-git-create-hook() {
 set -e
 set -o pipefail
 
-cat | DOKKU_ROOT="$DOKKU_ROOT" dokku git-hook $APP
+cat | DOKKU_ROOT="$DOKKU_ROOT" dokku git-hook "$APP"
 EOF
   chmod +x "$PRERECEIVE_HOOK"
 }

tests/unit/apps_1.bats

@@ -96,6 +96,26 @@ teardown() {
   echo "status: $status"
   assert_failure
 
+  run /bin/bash -c "dokku apps:create 'test;id'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_failure
+
+  run /bin/bash -c "dokku apps:create 'test\$cmd'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_failure
+
+  run /bin/bash -c "dokku apps:create 'test|cmd'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_failure
+
+  run /bin/bash -c "dokku apps:create 'test&cmd'"
+  echo "output: $output"
+  echo "status: $status"
+  assert_failure
+
   run /bin/bash -c "dokku --app $TEST_APP apps:create"
   echo "output: $output"
   echo "status: $status"