Skip to content
Merged
7 changes: 2 additions & 5 deletions apps/admin_audit/lib/Actions/Action.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,8 @@ public function log(string $text,
);
} 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;
Expand Down
71 changes: 0 additions & 71 deletions build/psalm-baseline-security.xml
Original file line number Diff line number Diff line change
@@ -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>
Expand All @@ -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>
Expand All @@ -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>
10 changes: 9 additions & 1 deletion lib/private/AppFramework/OCS/BaseResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ protected function renderResult(array $meta): string {
];

if ($this->format === 'json') {
return json_encode($response, JSON_HEX_TAG);
return $this->toJson($response);
}

$writer = new \XMLWriter();
Expand All @@ -111,6 +111,14 @@ protected function renderResult(array $meta): string {
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)) {
Expand Down
2 changes: 1 addition & 1 deletion lib/private/AppFramework/Utility/SimpleContainer.php
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ public function registerAlias($alias, $target) {
}, false);
}

/*
/**
* @param string $name
* @return string
*/
Expand Down
24 changes: 22 additions & 2 deletions lib/private/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,16 +65,36 @@ public function getKeys() {
*/
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
*
Expand Down
27 changes: 24 additions & 3 deletions lib/private/L10N/Factory.php
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,7 @@ public function get($app, $lang = null, $locale = null) {
$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);
Expand All @@ -130,6 +128,29 @@ public function get($app, $lang = null, $locale = null) {
});
}

/**
* 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
*
Expand Down
22 changes: 11 additions & 11 deletions lib/private/Setup/MySQL.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public function setupDatabase($username) {
/**
* @param \OC\DB\Connection $connection
*/
private function createDatabase($connection) {
private function createDatabase($connection): void {
try {
$name = $this->dbName;
$user = $this->dbUser;
Expand Down Expand Up @@ -91,23 +91,23 @@ private function createDatabase($connection) {
* @param IDBConnection $connection
* @throws \OC\DatabaseSetupException
*/
private function createDBUser($connection) {
private function createDBUser($connection): void {
try {
$name = $this->dbUser;
$password = $this->dbPassword;
// we need to create 2 accounts, one for global use and one for local user. if we don't specify the local one,
// 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.', [
Expand All @@ -119,7 +119,7 @@ private function createDBUser($connection) {
}

/**
* @param $username
* @param string $username
* @param IDBConnection $connection
*/
private function createSpecificUser($username, $connection): void {
Expand Down
20 changes: 1 addition & 19 deletions lib/private/SystemConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -116,24 +116,6 @@ public function __construct(
) {
}

/**
* 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
Expand Down Expand Up @@ -170,7 +152,7 @@ public function setValues(array $configs) {
* @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);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion lib/private/TaskProcessing/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -999,7 +999,7 @@ public function setTaskResult(int $id, ?string $error, ?array $result, bool $isU
$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);
Expand Down
5 changes: 3 additions & 2 deletions lib/private/legacy/OC_JSON.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ public static function checkAdminUser() {
* 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';
Expand All @@ -86,7 +85,6 @@ public static function error($data = []) {
* 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';
Expand All @@ -97,6 +95,9 @@ public static function success($data = []) {
/**
* 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);
Expand Down
12 changes: 10 additions & 2 deletions lib/private/legacy/OC_Template.php
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,15 @@ public static function printExceptionErrorPage($exception, $statusCode = 503) {
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");
Expand All @@ -323,7 +331,7 @@ private static function printPlainErrorPage(\Throwable $exception, bool $debug =
if ($debug) {
print("\n");
print($exception->getMessage() . ' ' . $exception->getFile() . ' at ' . $exception->getLine() . "\n");
print($exception->getTraceAsString());
print(self::fakeEscapeForPlainText($exception->getTraceAsString()));
}
}
}
3 changes: 3 additions & 0 deletions lib/public/AppFramework/Http/JSONResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ public function __construct(
* @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);
Expand Down
Loading