From 8f57a5076717280b2450ba2737d0c214bf45b314 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Thu, 13 Feb 2025 14:21:36 +0100 Subject: [PATCH 1/2] fix: Only keep allowed characters in appid, and flag the method as escaping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/App/AppManager.php | 19 +++++++++++++++++-- lib/public/App/IAppManager.php | 11 +++++++++-- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php index 84dde3be71262..544b304b9bd78 100644 --- a/lib/private/App/AppManager.php +++ b/lib/private/App/AppManager.php @@ -939,8 +939,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('/[^a-z0-9_]+/', '', $app); } } diff --git a/lib/public/App/IAppManager.php b/lib/public/App/IAppManager.php index 961823dee6e14..fa35819b779c8 100644 --- a/lib/public/App/IAppManager.php +++ b/lib/public/App/IAppManager.php @@ -309,10 +309,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 */ From 6e7c97ea1f6a8998249ef7365b3e67260ba72441 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= <91878298+come-nc@users.noreply.github.com> Date: Thu, 13 Feb 2025 16:22:21 +0100 Subject: [PATCH 2/2] fix: Also remove digits at the start and underscore on both ends of appid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Ferdinand Thiessen Signed-off-by: Côme Chilliet <91878298+come-nc@users.noreply.github.com> --- lib/private/App/AppManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php index 544b304b9bd78..067cc89d67429 100644 --- a/lib/private/App/AppManager.php +++ b/lib/private/App/AppManager.php @@ -956,6 +956,6 @@ public function isBackendRequired(string $backend): bool { */ public function cleanAppId(string $app): string { /* Only lowercase alphanumeric is allowed */ - return preg_replace('/[^a-z0-9_]+/', '', $app); + return preg_replace('/(^[0-9_]|[^a-z0-9_]+|_$)/', '', $app); } }