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

fix: Fix psalm taint errors
This commit is contained in:
Ferdinand Thiessen 2025-02-17 17:45:36 +01:00 committed by GitHub
commit 9edabfa21f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 87 additions and 118 deletions

View file

@ -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;

View file

@ -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>

View file

@ -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)) {

View file

@ -176,7 +176,7 @@ class SimpleContainer implements ArrayAccess, ContainerInterface, IContainer {
}, false);
}
/*
/**
* @param string $name
* @return string
*/

View file

@ -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
*

View file

@ -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
*

View file

@ -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 {

View file

@ -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);
}
/**

View file

@ -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);

View file

@ -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);

View file

@ -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()));
}
}
}

View file

@ -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);