Merge pull request #19346 from owncloud/drop-passwords-from-exception-log

Remove passwords from logged exception stack traces

lib/private/log.php

@@ -254,4 +254,25 @@ class Log implements ILogger {
 			call_user_func(array($logger, 'write'), $app, $message, $level);
 		}
 	}
+
+	/**
+	 * Logs an exception very detailed
+	 *
+	 * @param \Exception $exception
+	 * @param array $context
+	 * @return void
+	 * @since 8.2.0
+	 */
+	public function logException(\Exception $exception, array $context = array()) {
+		$exception = array(
+			'Exception' => get_class($exception),
+			'Message' => $exception->getMessage(),
+			'Code' => $exception->getCode(),
+			'Trace' => $exception->getTraceAsString(),
+			'File' => $exception->getFile(),
+			'Line' => $exception->getLine(),
+		);
+		$exception['Trace'] = preg_replace('!(login|checkPassword)\(.*\)!', '$1(*** username and password replaced ***)', $exception['Trace']);
+		$this->error('Exception: ' . json_encode($exception), $context);
+	}
 }

lib/public/ilogger.php

@@ -122,4 +122,14 @@ interface ILogger {
 	 * @since 7.0.0
 	 */
 	public function log($level, $message, array $context = array());
+
+	/**
+	 * Logs an exception very detailed
+	 *
+	 * @param \Exception $exception
+	 * @param array $context
+	 * @return void
+	 * @since 8.2.0
+	 */
+	public function logException(\Exception $exception, array $context = array());
 }

lib/public/util.php

@@ -158,17 +158,10 @@ class Util {
 	 * @param \Exception $ex exception to log
 	 * @param int $level log level, defaults to \OCP\Util::FATAL
 	 * @since ....0.0 - parameter $level was added in 7.0.0
+	 * @deprecated 8.2.0 use logException of \OCP\ILogger
 	 */
 	public static function logException( $app, \Exception $ex, $level = \OCP\Util::FATAL ) {
-		$exception = array(
-			'Exception' => get_class($ex),
-			'Message' => $ex->getMessage(),
-			'Code' => $ex->getCode(),
-			'Trace' => $ex->getTraceAsString(),
-			'File' => $ex->getFile(),
-			'Line' => $ex->getLine(),
-		);
-		\OCP\Util::writeLog($app, 'Exception: ' . json_encode($exception), $level);
+		\OC::$server->getLogger()->logException($ex, ['app' => $app]);
 	}
 
 	/**

tests/lib/logger.php

@@ -63,4 +63,48 @@ class Logger extends TestCase {
 	public static function write($app, $message, $level) {
 		self::$logs[]= "$level $message";
 	}
+
+	public function userAndPasswordData() {
+		return [
+			['abc', 'def'],
+			['mySpecialUsername', 'MySuperSecretPassword'],
+			['my-user', '324324()#ä234'],
+			['my-user', ')qwer'],
+			['my-user', 'qwer)asdf'],
+			['my-user', 'qwer)'],
+			['my-user', '(qwer'],
+			['my-user', 'qwer(asdf'],
+			['my-user', 'qwer('],
+		];
+	}
+
+	/**
+	 * @dataProvider userAndPasswordData
+	 */
+	public function testDetectlogin($user, $password) {
+		$e = new \Exception('test');
+		$this->logger->logException($e);
+
+		$logLines = $this->getLogs();
+		foreach($logLines as $logLine) {
+			$this->assertNotContains($user, $logLine);
+			$this->assertNotContains($password, $logLine);
+			$this->assertContains('login(*** username and password replaced ***)', $logLine);
+		}
+	}
+
+	/**
+	 * @dataProvider userAndPasswordData
+	 */
+	public function testDetectcheckPassword($user, $password) {
+		$e = new \Exception('test');
+		$this->logger->logException($e);
+		$logLines = $this->getLogs();
+
+		foreach($logLines as $logLine) {
+			$this->assertNotContains($user, $logLine);
+			$this->assertNotContains($password, $logLine);
+			$this->assertContains('checkPassword(*** username and password replaced ***)', $logLine);
+		}
+	}
 }