From a20c497f3d79f9dbacc1e976c5603ef9bdc977e8 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 b6f7f9b13b798..3f2e31f08ee27 100644 --- 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('/[^a-z0-9_]+/', '', $app); } } diff --git a/lib/public/App/IAppManager.php b/lib/public/App/IAppManager.php index 110bcacf396be..f16b188e6b6a8 100644 --- 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 */ From d66c6124497c78c8c8083ddbd953f706e840614a 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 3f2e31f08ee27..12282af087313 100644 --- a/lib/private/App/AppManager.php +++ b/lib/private/App/AppManager.php @@ -943,6 +943,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); } }