diff --git a/lib/private/Log/ExceptionSerializer.php b/lib/private/Log/ExceptionSerializer.php index fb63ddf0a0e0b..1eba042fa15ee 100644 --- a/lib/private/Log/ExceptionSerializer.php +++ b/lib/private/Log/ExceptionSerializer.php @@ -42,6 +42,8 @@ use OCA\Encryption\Session; class ExceptionSerializer { + public const SENSITIVE_VALUE_PLACEHOLDER = '*** sensitive parameters replaced ***'; + public const methodsWithSensitiveParameters = [ // Session/User 'completeLogin', @@ -180,7 +182,7 @@ private function editTrace(array &$sensitiveValues, array $traceLine): array { 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 @@ private function filterTrace(array $trace) { } 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 0000000000000..ea9a52179f5c9 --- /dev/null +++ b/tests/lib/Log/ExceptionSerializerTest.php @@ -0,0 +1,68 @@ + + * + * @author Arthur Schiwon + * + * @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 . + * + */ + +namespace lib\Log; + +use OC\Log\ExceptionSerializer; +use OC\SystemConfig; +use Test\TestCase; + +class ExceptionSerializerTest extends TestCase { + private $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]); + } + } +}