diff options
author | blizzz <blizzz@arthur-schiwon.de> | 2022-06-17 15:22:32 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-06-17 15:22:32 +0300 |
commit | a71d33ad91695ceb3551a1ee19d69407b339766a (patch) | |
tree | 03c5ee5152713b5c5c976528046558ba490d5c65 | |
parent | bab0753507f9eebb5f4240aa1cdc64dd4ed26d32 (diff) | |
parent | 6b8d6de4fc99fd7c0850603b53f3a0fadadebe3b (diff) |
Merge pull request #32904 from nextcloud/backport/32898/stable24
[stable24] Fix logger overwriting vars in some circumstances
-rw-r--r-- | lib/private/Log/ExceptionSerializer.php | 12 | ||||
-rw-r--r-- | tests/lib/Log/ExceptionSerializerTest.php | 68 |
2 files changed, 76 insertions, 4 deletions
diff --git a/lib/private/Log/ExceptionSerializer.php b/lib/private/Log/ExceptionSerializer.php index dab134b26a4..3c3ff95f8e1 100644 --- a/lib/private/Log/ExceptionSerializer.php +++ b/lib/private/Log/ExceptionSerializer.php @@ -42,6 +42,8 @@ use OCA\Encryption\Session; use OCP\HintException; class ExceptionSerializer { + public const SENSITIVE_VALUE_PLACEHOLDER = '*** sensitive parameters replaced ***'; + public const methodsWithSensitiveParameters = [ // Session/User 'completeLogin', @@ -180,7 +182,7 @@ class ExceptionSerializer { if (isset($traceLine['args'])) { $sensitiveValues = array_merge($sensitiveValues, $traceLine['args']); } - $traceLine['args'] = ['*** sensitive parameters replaced ***']; + $traceLine['args'] = [self::SENSITIVE_VALUE_PLACEHOLDER]; return $traceLine; } @@ -208,14 +210,16 @@ class ExceptionSerializer { } private function removeValuesFromArgs($args, $values) { - foreach ($args as &$arg) { + $workArgs = []; + foreach ($args as $arg) { if (in_array($arg, $values, true)) { - $arg = '*** sensitive parameter replaced ***'; + $arg = self::SENSITIVE_VALUE_PLACEHOLDER; } elseif (is_array($arg)) { $arg = $this->removeValuesFromArgs($arg, $values); } + $workArgs[] = $arg; } - return $args; + return $workArgs; } private function encodeTrace($trace) { diff --git a/tests/lib/Log/ExceptionSerializerTest.php b/tests/lib/Log/ExceptionSerializerTest.php new file mode 100644 index 00000000000..70ac80d13e3 --- /dev/null +++ b/tests/lib/Log/ExceptionSerializerTest.php @@ -0,0 +1,68 @@ +<?php + +declare(strict_types=1); + +/** + * @copyright Copyright (c) 2021 Arthur Schiwon <blizzz@arthur-schiwon.de> + * + * @author Arthur Schiwon <blizzz@arthur-schiwon.de> + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see <https://www.gnu.org/licenses/>. + * + */ + +namespace lib\Log; + +use OC\Log\ExceptionSerializer; +use OC\SystemConfig; +use Test\TestCase; + +class ExceptionSerializerTest extends TestCase { + private ExceptionSerializer $serializer; + + public function setUp(): void { + parent::setUp(); + + $config = $this->createMock(SystemConfig::class); + $this->serializer = new ExceptionSerializer($config); + } + + private function emit($arguments) { + \call_user_func_array([$this, 'bind'], $arguments); + } + + private function bind(array &$myValues): void { + throw new \Exception('my exception'); + } + + /** + * this test ensures that the serializer does not overwrite referenced + * variables. It is crafted after a scenario we experienced: the DAV server + * emitting the "validateTokens" event, of which later on a handled + * exception was passed to the logger. The token was replaced, the original + * variable overwritten. + */ + public function testSerializer() { + try { + $secret = ['Secret']; + $this->emit([&$secret]); + } catch (\Exception $e) { + $serializedData = $this->serializer->serializeException($e); + $this->assertSame(['Secret'], $secret); + $this->assertSame(ExceptionSerializer::SENSITIVE_VALUE_PLACEHOLDER, $serializedData['Trace'][0]['args'][0]); + } + } +} |