Merge pull request #50800 from nextcloud/fix/fix-psalm-taint-errors

fix: Fix psalm taint errors

12 files changed, 87 insertions, 118 deletions

diff --git a/apps/admin_audit/lib/Actions/Action.php b/apps/admin_audit/lib/Actions/Action.php
index 2566025a8ce..acd415d82ea 100644
--- a/apps/admin_audit/lib/Actions/Action.php
+++ b/apps/admin_audit/lib/Actions/Action.php
@@ -37,11 +37,8 @@ class Action {
 					);
 				} else {
 					$this->logger->critical(
-						sprintf(
-							'$params["' . $element . '"] was missing. Transferred value: %s',
-							print_r($params, true)
-						),
-						['app' => 'admin_audit']
+						'$params["' . $element . '"] was missing. Transferred value: {params}',
+						['app' => 'admin_audit', 'params' => $params]
 					);
 				}
 				return;
diff --git a/build/psalm-baseline-security.xml b/build/psalm-baseline-security.xml
index 8740e96d78b..2777f4e7734 100644
--- a/build/psalm-baseline-security.xml
+++ b/build/psalm-baseline-security.xml
@@ -1,10 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <files psalm-version="5.26.1@d747f6500b38ac4f7dfc5edbcae6e4b637d7add0">
-  <file src="apps/admin_audit/lib/Actions/Action.php">
-    <TaintedHtml>
-      <code><![CDATA[$params]]></code>
-    </TaintedHtml>
-  </file>
   <file src="apps/files_external/lib/Config/ConfigAdapter.php">
     <TaintedCallable>
       <code><![CDATA[$objectClass]]></code>
@@ -16,40 +11,11 @@
       <code><![CDATA[$imageFile]]></code>
     </TaintedFile>
   </file>
-  <file src="lib/base.php">
-    <TaintedHeader>
-      <code><![CDATA['Location: ' . $url]]></code>
-      <code><![CDATA['Location: ' . \OC::$WEBROOT . '/']]></code>
-    </TaintedHeader>
-  </file>
   <file src="lib/private/Config.php">
     <TaintedHtml>
       <code><![CDATA[$this->cache]]></code>
     </TaintedHtml>
   </file>
-  <file src="lib/private/EventSource.php">
-    <TaintedHeader>
-      <code><![CDATA['Location: ' . \OC::$WEBROOT]]></code>
-    </TaintedHeader>
-  </file>
-  <file src="lib/private/Http/CookieHelper.php">
-    <TaintedHeader>
-      <code><![CDATA[$header]]></code>
-    </TaintedHeader>
-  </file>
-  <file src="lib/private/Installer.php">
-    <TaintedFile>
-      <code><![CDATA[$baseDir]]></code>
-    </TaintedFile>
-  </file>
-  <file src="lib/private/OCS/ApiHelper.php">
-    <TaintedHtml>
-      <code><![CDATA[$body]]></code>
-    </TaintedHtml>
-    <TaintedTextWithQuotes>
-      <code><![CDATA[$body]]></code>
-    </TaintedTextWithQuotes>
-  </file>
   <file src="lib/private/Route/Router.php">
     <TaintedCallable>
       <code><![CDATA[$appNameSpace . '\\Controller\\' . basename($file->getPathname(), '.php')]]></code>
@@ -70,46 +36,9 @@
       <code><![CDATA[$sqliteFile]]></code>
     </TaintedFile>
   </file>
-  <file src="lib/private/legacy/OC_Helper.php">
-    <TaintedFile>
-      <code><![CDATA[$dest]]></code>
-      <code><![CDATA[$dest]]></code>
-      <code><![CDATA[$dir]]></code>
-      <code><![CDATA[$dir]]></code>
-    </TaintedFile>
-  </file>
-  <file src="lib/private/legacy/OC_JSON.php">
-    <TaintedHeader>
-      <code><![CDATA['Location: ' . \OC::$WEBROOT]]></code>
-    </TaintedHeader>
-  </file>
-  <file src="lib/private/legacy/OC_Template.php">
-    <TaintedHtml>
-      <code><![CDATA[$exception->getTraceAsString()]]></code>
-    </TaintedHtml>
-    <TaintedTextWithQuotes>
-      <code><![CDATA[$exception->getTraceAsString()]]></code>
-    </TaintedTextWithQuotes>
-  </file>
   <file src="lib/public/DB/QueryBuilder/IQueryBuilder.php">
     <TaintedSql>
       <code><![CDATA[$column]]></code>
     </TaintedSql>
   </file>
-  <file src="lib/public/IDBConnection.php">
-    <TaintedSql>
-      <code><![CDATA[$sql]]></code>
-      <code><![CDATA[$sql]]></code>
-      <code><![CDATA[$sql]]></code>
-      <code><![CDATA[$sql]]></code>
-    </TaintedSql>
-  </file>
-  <file src="ocs-provider/index.php">
-    <TaintedHtml>
-      <code><![CDATA[$controller->buildProviderList()->render()]]></code>
-    </TaintedHtml>
-    <TaintedTextWithQuotes>
-      <code><![CDATA[$controller->buildProviderList()->render()]]></code>
-    </TaintedTextWithQuotes>
-  </file>
 </files>
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)) {
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
 	 */
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,17 +65,37 @@ 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
 	 *
 	 * @param array $configs Associative array with `key => value` pairs
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);
@@ -131,6 +129,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
 	 *
 	 * @param string|null $appId App id or null for core
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 {
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
@@ -117,24 +117,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);
 	}
 
 	/**
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);
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/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()));
 		}
 	}
 }
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);