From db8e7ce8b95c882c876f932296f25ec08883a1d3 Mon Sep 17 00:00:00 2001
From: Morris Jobke <hey@morrisjobke.de>
Date: Thu, 24 Sep 2015 17:01:31 +0200
Subject: [PATCH] Remove passwords from logged exception stack traces

* fixed #16318
* create logException in ILogger
* add unit tests
---
 lib/private/log.php    | 21 ++++++++++++++++++++
 lib/public/ilogger.php | 10 ++++++++++
 lib/public/util.php    | 11 ++---------
 tests/lib/logger.php   | 44 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 77 insertions(+), 9 deletions(-)

diff --git a/lib/private/log.php b/lib/private/log.php
index 3c0e7b45d1..1b26ece762 100644
--- a/lib/private/log.php
+++ b/lib/private/log.php
@@ -241,4 +241,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);
+	}
 }
diff --git a/lib/public/ilogger.php b/lib/public/ilogger.php
index 43b1ef70e5..27a5d63dfd 100644
--- a/lib/public/ilogger.php
+++ b/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());
 }
diff --git a/lib/public/util.php b/lib/public/util.php
index c32668b14a..652df5192c 100644
--- a/lib/public/util.php
+++ b/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]);
 	}
 
 	/**
diff --git a/tests/lib/logger.php b/tests/lib/logger.php
index c8566988cf..9c9cd9e672 100644
--- a/tests/lib/logger.php
+++ b/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);
+		}
+	}
 }