From 579a337750c85bab1f1e6d798c10cbb012f3f819 Mon Sep 17 00:00:00 2001 From: Côme Chilliet Date: Thu, 13 Feb 2025 18:06:43 +0100 Subject: fix: Fix psalm taint error in L10N factory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/L10N/Factory.php | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/private/L10N/Factory.php b/lib/private/L10N/Factory.php index eb84f264f5f..5645693f8d9 100644 --- a/lib/private/L10N/Factory.php +++ b/lib/private/L10N/Factory.php @@ -108,9 +108,7 @@ class Factory implements IFactory { $locale = $forceLocale; } - if ($lang === null || !$this->languageExists($app, $lang)) { - $lang = $this->findLanguage($app); - } + $lang = $this->validateLanguage($app, $lang); if ($locale === null || !$this->localeExists($locale)) { $locale = $this->findLocale($lang); @@ -130,6 +128,29 @@ class Factory implements IFactory { }); } + /** + * Check that $lang is an existing language and not null, otherwise return the language to use instead + * + * @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 + */ + private function validateLanguage(string $app, ?string $lang): string { + if ($lang === null || !$this->languageExists($app, $lang)) { + return $this->findLanguage($app); + } else { + return $lang; + } + } + /** * Find the best language * -- cgit v1.2.3 From fec865cc29ee0bc54dbd29c07e8cbe3d477bfca2 Mon Sep 17 00:00:00 2001 From: Côme Chilliet Date: Mon, 17 Feb 2025 11:16:27 +0100 Subject: chore: Correctly flag json encoding methods as escaping html and quotes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Especially with JSON_HEX_TAG it’s perfectly fine to echo JSON, and we only use it in JSON output anyway. Signed-off-by: Côme Chilliet --- build/psalm-baseline-security.xml | 8 -------- lib/private/legacy/OC_JSON.php | 5 +++-- lib/public/AppFramework/Http/JSONResponse.php | 3 +++ 3 files changed, 6 insertions(+), 10 deletions(-) (limited to 'lib') diff --git a/build/psalm-baseline-security.xml b/build/psalm-baseline-security.xml index 8740e96d78b..f15718796c2 100644 --- a/build/psalm-baseline-security.xml +++ b/build/psalm-baseline-security.xml @@ -104,12 +104,4 @@ - - - buildProviderList()->render()]]> - - - buildProviderList()->render()]]> - - diff --git a/lib/private/legacy/OC_JSON.php b/lib/private/legacy/OC_JSON.php index d2b85951123..6daef18dd61 100644 --- a/lib/private/legacy/OC_JSON.php +++ b/lib/private/legacy/OC_JSON.php @@ -74,7 +74,6 @@ class OC_JSON { * Send json error msg * @deprecated 12.0.0 Use a AppFramework JSONResponse instead * @suppress PhanDeprecatedFunction - * @psalm-taint-escape html */ public static function error($data = []) { $data['status'] = 'error'; @@ -86,7 +85,6 @@ class OC_JSON { * Send json success msg * @deprecated 12.0.0 Use a AppFramework JSONResponse instead * @suppress PhanDeprecatedFunction - * @psalm-taint-escape html */ public static function success($data = []) { $data['status'] = 'success'; @@ -97,6 +95,9 @@ class OC_JSON { /** * Encode JSON * @deprecated 12.0.0 Use a AppFramework JSONResponse instead + * + * @psalm-taint-escape has_quotes + * @psalm-taint-escape html */ private static function encode($data) { return json_encode($data, JSON_HEX_TAG); diff --git a/lib/public/AppFramework/Http/JSONResponse.php b/lib/public/AppFramework/Http/JSONResponse.php index efcf79d5e87..a226e29a1b5 100644 --- a/lib/public/AppFramework/Http/JSONResponse.php +++ b/lib/public/AppFramework/Http/JSONResponse.php @@ -58,6 +58,9 @@ class JSONResponse extends Response { * @return string the rendered json * @since 6.0.0 * @throws \Exception If data could not get encoded + * + * @psalm-taint-escape has_quotes + * @psalm-taint-escape html */ public function render() { return json_encode($this->data, JSON_HEX_TAG | JSON_THROW_ON_ERROR | $this->encodeFlags, 2048); -- cgit v1.2.3 From 85fbd3eb0ab1650f9385acd869d5f4ab21006a8a Mon Sep 17 00:00:00 2001 From: Côme Chilliet Date: Mon, 17 Feb 2025 11:59:23 +0100 Subject: fix: Work around psalm taint false-positive by not using var_export MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit var_export is listed as a taint sink because it may output stuff depending on the parameters. It was not the case here, but we can simply json_encode the result by passing it as context to the logger method rather than using var_export. Signed-off-by: Côme Chilliet --- lib/private/TaskProcessing/Manager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/private/TaskProcessing/Manager.php b/lib/private/TaskProcessing/Manager.php index 07e643ab004..0582d801e3d 100644 --- a/lib/private/TaskProcessing/Manager.php +++ b/lib/private/TaskProcessing/Manager.php @@ -999,7 +999,7 @@ class Manager implements IManager { $task->setEndedAt(time()); $error = 'The task was processed successfully but the provider\'s output doesn\'t pass validation against the task type\'s outputShape spec and/or the provider\'s own optionalOutputShape spec'; $task->setErrorMessage($error); - $this->logger->error($error . ' Output was: ' . var_export($result, true), ['exception' => $e]); + $this->logger->error($error, ['exception' => $e, 'output' => $result]); } catch (NotPermittedException $e) { $task->setProgress(1); $task->setStatus(Task::STATUS_FAILED); -- cgit v1.2.3 From aac79bad9b70716514345370cb98066aa11138a7 Mon Sep 17 00:00:00 2001 From: Côme Chilliet Date: Mon, 17 Feb 2025 12:35:18 +0100 Subject: fix: Move config.php taint trust upstream directly in OC\Config class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This solves some false-positive psalm taint errors Signed-off-by: Côme Chilliet --- build/psalm-baseline-security.xml | 34 ---------------------------------- lib/private/Config.php | 24 ++++++++++++++++++++++-- lib/private/SystemConfig.php | 20 +------------------- 3 files changed, 23 insertions(+), 55 deletions(-) (limited to 'lib') diff --git a/build/psalm-baseline-security.xml b/build/psalm-baseline-security.xml index 4df4cd7d687..45f0e54f648 100644 --- a/build/psalm-baseline-security.xml +++ b/build/psalm-baseline-security.xml @@ -11,32 +11,11 @@ - - - - - - cache]]> - - - - - - - - - - - - - - - @@ -65,19 +44,6 @@ - - - - - - - - - - - - - getTraceAsString()]]> diff --git a/lib/private/Config.php b/lib/private/Config.php index 0e8d07955af..3ec21df9f7c 100644 --- a/lib/private/Config.php +++ b/lib/private/Config.php @@ -65,16 +65,36 @@ class Config { */ public function getValue($key, $default = null) { if (isset($this->envCache[$key])) { - return $this->envCache[$key]; + return self::trustSystemConfig($this->envCache[$key]); } if (isset($this->cache[$key])) { - return $this->cache[$key]; + return self::trustSystemConfig($this->cache[$key]); } return $default; } + /** + * Since system config is admin controlled, we can tell psalm to ignore any taint + * + * @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 + * @psalm-pure + */ + public static function trustSystemConfig(mixed $value): mixed { + return $value; + } + /** * Sets and deletes values and writes the config.php * diff --git a/lib/private/SystemConfig.php b/lib/private/SystemConfig.php index d3fd1f2ab04..57777b06ed6 100644 --- a/lib/private/SystemConfig.php +++ b/lib/private/SystemConfig.php @@ -116,24 +116,6 @@ class SystemConfig { ) { } - /** - * Since system config is admin controlled, we can tell psalm to ignore any taint - * - * @psalm-taint-escape sql - * @psalm-taint-escape html - * @psalm-taint-escape ldap - * @psalm-taint-escape callable - * @psalm-taint-escape file - * @psalm-taint-escape ssrf - * @psalm-taint-escape cookie - * @psalm-taint-escape header - * @psalm-taint-escape has_quotes - * @psalm-pure - */ - public static function trustSystemConfig(mixed $value): mixed { - return $value; - } - /** * Lists all available config keys * @return array an array of key names @@ -170,7 +152,7 @@ class SystemConfig { * @return mixed the value or $default */ public function getValue($key, $default = '') { - return $this->trustSystemConfig($this->config->getValue($key, $default)); + return $this->config->getValue($key, $default); } /** -- cgit v1.2.3 From fa108d5b5414d8fdfa1e5eecd9a7d871d58f4b28 Mon Sep 17 00:00:00 2001 From: Côme Chilliet Date: Mon, 17 Feb 2025 14:13:23 +0100 Subject: fix: Correctly tag json encoding in BaseResponse to fix false-positive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …in psalm taint analysis Signed-off-by: Côme Chilliet --- build/psalm-baseline-security.xml | 8 -------- lib/private/AppFramework/OCS/BaseResponse.php | 10 +++++++++- 2 files changed, 9 insertions(+), 9 deletions(-) (limited to 'lib') diff --git a/build/psalm-baseline-security.xml b/build/psalm-baseline-security.xml index 45f0e54f648..c7b083b22c5 100644 --- a/build/psalm-baseline-security.xml +++ b/build/psalm-baseline-security.xml @@ -16,14 +16,6 @@ cache]]> - - - - - - - - getPathname(), '.php')]]> diff --git a/lib/private/AppFramework/OCS/BaseResponse.php b/lib/private/AppFramework/OCS/BaseResponse.php index cc7f7845760..5929a3993ec 100644 --- a/lib/private/AppFramework/OCS/BaseResponse.php +++ b/lib/private/AppFramework/OCS/BaseResponse.php @@ -99,7 +99,7 @@ abstract class BaseResponse extends Response { ]; if ($this->format === 'json') { - return json_encode($response, JSON_HEX_TAG); + return $this->toJson($response); } $writer = new \XMLWriter(); @@ -111,6 +111,14 @@ abstract class BaseResponse extends Response { return $writer->outputMemory(true); } + /** + * @psalm-taint-escape has_quotes + * @psalm-taint-escape html + */ + protected function toJson(array $array): string { + return \json_encode($array, \JSON_HEX_TAG); + } + protected function toXML(array $array, \XMLWriter $writer): void { foreach ($array as $k => $v) { if ($k === '@attributes' && is_array($v)) { -- cgit v1.2.3 From 7c907223d2c61df3a3ee3ec25cf4d48f058c5751 Mon Sep 17 00:00:00 2001 From: Côme Chilliet Date: Mon, 17 Feb 2025 14:28:30 +0100 Subject: fix: Fix psalm taint false-positive by escaping trusted input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- build/psalm-baseline-security.xml | 8 -------- lib/private/Setup/MySQL.php | 22 +++++++++++----------- 2 files changed, 11 insertions(+), 19 deletions(-) (limited to 'lib') diff --git a/build/psalm-baseline-security.xml b/build/psalm-baseline-security.xml index c7b083b22c5..d31034d538d 100644 --- a/build/psalm-baseline-security.xml +++ b/build/psalm-baseline-security.xml @@ -49,12 +49,4 @@ - - - - - - - - diff --git a/lib/private/Setup/MySQL.php b/lib/private/Setup/MySQL.php index 2708ada31c1..6dd9855d851 100644 --- a/lib/private/Setup/MySQL.php +++ b/lib/private/Setup/MySQL.php @@ -59,7 +59,7 @@ class MySQL extends AbstractDatabase { /** * @param \OC\DB\Connection $connection */ - private function createDatabase($connection) { + private function createDatabase($connection): void { try { $name = $this->dbName; $user = $this->dbUser; @@ -91,7 +91,7 @@ class MySQL extends AbstractDatabase { * @param IDBConnection $connection * @throws \OC\DatabaseSetupException */ - private function createDBUser($connection) { + private function createDBUser($connection): void { try { $name = $this->dbUser; $password = $this->dbPassword; @@ -99,15 +99,15 @@ class MySQL extends AbstractDatabase { // the anonymous user would take precedence when there is one. if ($connection->getDatabasePlatform() instanceof Mysql80Platform) { - $query = "CREATE USER '$name'@'localhost' IDENTIFIED WITH mysql_native_password BY '$password'"; - $connection->executeUpdate($query); - $query = "CREATE USER '$name'@'%' IDENTIFIED WITH mysql_native_password BY '$password'"; - $connection->executeUpdate($query); + $query = "CREATE USER ?@'localhost' IDENTIFIED WITH mysql_native_password BY ?"; + $connection->executeUpdate($query, [$name,$password]); + $query = "CREATE USER ?@'%' IDENTIFIED WITH mysql_native_password BY ?"; + $connection->executeUpdate($query, [$name,$password]); } else { - $query = "CREATE USER '$name'@'localhost' IDENTIFIED BY '$password'"; - $connection->executeUpdate($query); - $query = "CREATE USER '$name'@'%' IDENTIFIED BY '$password'"; - $connection->executeUpdate($query); + $query = "CREATE USER ?@'localhost' IDENTIFIED BY ?"; + $connection->executeUpdate($query, [$name,$password]); + $query = "CREATE USER ?@'%' IDENTIFIED BY ?"; + $connection->executeUpdate($query, [$name,$password]); } } catch (\Exception $ex) { $this->logger->error('Database user creation failed.', [ @@ -119,7 +119,7 @@ class MySQL extends AbstractDatabase { } /** - * @param $username + * @param string $username * @param IDBConnection $connection */ private function createSpecificUser($username, $connection): void { -- cgit v1.2.3 From 640dbd0b5e38ef603c4edcc646ed7df8117c9963 Mon Sep 17 00:00:00 2001 From: Côme Chilliet Date: Mon, 17 Feb 2025 15:00:58 +0100 Subject: fix: Fix false-positive psalm taint errors when outputting plain text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- build/psalm-baseline-security.xml | 8 -------- lib/private/legacy/OC_Template.php | 12 ++++++++++-- 2 files changed, 10 insertions(+), 10 deletions(-) (limited to 'lib') diff --git a/build/psalm-baseline-security.xml b/build/psalm-baseline-security.xml index d31034d538d..2777f4e7734 100644 --- a/build/psalm-baseline-security.xml +++ b/build/psalm-baseline-security.xml @@ -36,14 +36,6 @@ - - - getTraceAsString()]]> - - - getTraceAsString()]]> - - diff --git a/lib/private/legacy/OC_Template.php b/lib/private/legacy/OC_Template.php index 1026e536b97..af363e0a41e 100644 --- a/lib/private/legacy/OC_Template.php +++ b/lib/private/legacy/OC_Template.php @@ -313,7 +313,15 @@ class OC_Template extends \OC\Template\Base { die(); } - private static function printPlainErrorPage(\Throwable $exception, bool $debug = false) { + /** + * @psalm-taint-escape has_quotes + * @psalm-taint-escape html + */ + private static function fakeEscapeForPlainText(string $str): string { + return $str; + } + + private static function printPlainErrorPage(\Throwable $exception, bool $debug = false): void { header('Content-Type: text/plain; charset=utf-8'); print("Internal Server Error\n\n"); print("The server encountered an internal error and was unable to complete your request.\n"); @@ -323,7 +331,7 @@ class OC_Template extends \OC\Template\Base { if ($debug) { print("\n"); print($exception->getMessage() . ' ' . $exception->getFile() . ' at ' . $exception->getLine() . "\n"); - print($exception->getTraceAsString()); + print(self::fakeEscapeForPlainText($exception->getTraceAsString())); } } } -- cgit v1.2.3 From c1c59f9a6c5b92ef09e3cf182b198de3633f4aff Mon Sep 17 00:00:00 2001 From: Côme Chilliet Date: Mon, 17 Feb 2025 15:01:33 +0100 Subject: chore: Add missing star in phpdoc comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- lib/private/AppFramework/Utility/SimpleContainer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/private/AppFramework/Utility/SimpleContainer.php b/lib/private/AppFramework/Utility/SimpleContainer.php index 56de4a34cf6..24918992ea3 100644 --- a/lib/private/AppFramework/Utility/SimpleContainer.php +++ b/lib/private/AppFramework/Utility/SimpleContainer.php @@ -176,7 +176,7 @@ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer { }, false); } - /* + /** * @param string $name * @return string */ -- cgit v1.2.3