Merge pull request #50799 from nextcloud/backport/50794/stable31

[stable31] fix: Only keep allowed characters in appid, and flag the method as escaping
nextcloud · Feb 19, 2025 · 747fbf6 · 747fbf6
2 parents f0a229c + a9ad23e
commit 747fbf6
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 4 deletions.
diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php
@@ -926,8 +926,23 @@ public function isBackendRequired(string $backend): bool {
 		return false;
 	}
 
+	/**
+	 * Clean the appId from forbidden characters
+	 *
+	 * @psalm-taint-escape callable
+	 * @psalm-taint-escape cookie
+	 * @psalm-taint-escape file
+	 * @psalm-taint-escape has_quotes
+	 * @psalm-taint-escape header
+	 * @psalm-taint-escape html
+	 * @psalm-taint-escape include
+	 * @psalm-taint-escape ldap
+	 * @psalm-taint-escape shell
+	 * @psalm-taint-escape sql
+	 * @psalm-taint-escape unserialize
+	 */
 	public function cleanAppId(string $app): string {
-		// FIXME should list allowed characters instead
-		return str_replace(['<', '>', '"', "'", '\0', '/', '\\', '..'], '', $app);
+		/* Only lowercase alphanumeric is allowed */
+		return preg_replace('/(^[0-9_]|[^a-z0-9_]+|_$)/', '', $app);
 	}
 }
diff --git a/lib/public/App/IAppManager.php b/lib/public/App/IAppManager.php
@@ -292,10 +292,17 @@ public function isBackendRequired(string $backend): bool;
 	/**
 	 * Clean the appId from forbidden characters
 	 *
+	 * @psalm-taint-escape callable
+	 * @psalm-taint-escape cookie
 	 * @psalm-taint-escape file
-	 * @psalm-taint-escape include
-	 * @psalm-taint-escape html
 	 * @psalm-taint-escape has_quotes
+	 * @psalm-taint-escape header
+	 * @psalm-taint-escape html
+	 * @psalm-taint-escape include
+	 * @psalm-taint-escape ldap
+	 * @psalm-taint-escape shell
+	 * @psalm-taint-escape sql
+	 * @psalm-taint-escape unserialize
 	 *
 	 * @since 31.0.0
 	 */