Do not show exception to the end-user

Log the error instead of potentially leaking sensitive information

4 files changed, 30 insertions, 34 deletions

diff --git a/lib/private/hintexception.php b/lib/private/hintexception.php
index 3934ae2a4c2..3a8361e9e80 100644
--- a/lib/private/hintexception.php
+++ b/lib/private/hintexception.php
@@ -12,7 +12,7 @@ class HintException extends \Exception {
 
 	private $hint;
 
-	public function __construct($message, $hint = '', $code = 0, Exception $previous = null) {
+	public function __construct($message, $hint = '', $code = 0, \Exception $previous = null) {
 		$this->hint = $hint;
 		parent::__construct($message, $code, $previous);
 	}
diff --git a/lib/private/log/owncloud.php b/lib/private/log/owncloud.php
index 08d0b7d5f93..d257bd12d2a 100644
--- a/lib/private/log/owncloud.php
+++ b/lib/private/log/owncloud.php
@@ -28,7 +28,6 @@
 
 class OC_Log_Owncloud {
 	static protected $logFile;
-	static protected $reqId;
 
 	/**
 	 * Init class data
@@ -69,19 +68,17 @@ class OC_Log_Owncloud {
 				$timezone = new DateTimeZone('UTC');
 			}
 			$time = new DateTime(null, $timezone);
+			$reqId = \OC_Request::getRequestID();
+			$remoteAddr = \OC_Request::getRemoteAddress();
 			// remove username/passwords from URLs before writing the to the log file
 			$time = $time->format($format);
 			if($minLevel == OC_Log::DEBUG) {
-				if(empty(self::$reqId)) {
-					self::$reqId = uniqid();
-				}
-				$reqId = self::$reqId;
 				$url = isset($_SERVER['REQUEST_URI']) ? $_SERVER['REQUEST_URI'] : '--';
 				$method = isset($_SERVER['REQUEST_METHOD']) ? $_SERVER['REQUEST_METHOD'] : '--';
-				$entry = compact('reqId', 'app', 'message', 'level', 'time', 'method', 'url');
+				$entry = compact('reqId', 'remoteAddr', 'app', 'message', 'level', 'time', 'method', 'url');
 			}
 			else {
-				$entry = compact('app', 'message', 'level', 'time');
+				$entry = compact('reqId', 'remoteAddr', 'app', 'message', 'level', 'time');
 			}
 			$entry = json_encode($entry);
 			$handle = @fopen(self::$logFile, 'a');
diff --git a/lib/private/request.php b/lib/private/request.php
index b063c1f5967..fa446837a97 100755
--- a/lib/private/request.php
+++ b/lib/private/request.php
@@ -14,6 +14,7 @@ class OC_Request {
 	const USER_AGENT_FREEBOX = '#^Mozilla/5\.0$#';
 
 	const REGEX_LOCALHOST = '/^(127\.0\.0\.1|localhost)(:[0-9]+|)$/';
+	static protected $reqId;
 
 	/**
 	 * Returns the remote address, if the connection came from a trusted proxy and `forwarded_for_headers` has been configured
@@ -22,7 +23,7 @@ class OC_Request {
 	 * @return string IP address
 	 */
 	public static function getRemoteAddress() {
-		$remoteAddress = $_SERVER['REMOTE_ADDR'];
+		$remoteAddress = isset($_SERVER['REMOTE_ADDR']) ? $_SERVER['REMOTE_ADDR'] : '';
 		$trustedProxies = \OC::$server->getConfig()->getSystemValue('trusted_proxies', array());
 
 		if(is_array($trustedProxies) && in_array($remoteAddress, $trustedProxies)) {
@@ -44,6 +45,17 @@ class OC_Request {
 	}
 
 	/**
+	 * Returns an ID for the request, value is not guaranteed to be unique and is mostly meant for logging
+	 * @return string
+	 */
+	public static function getRequestID() {
+		if(self::$reqId === null) {
+			self::$reqId = hash('md5', microtime().\OC::$server->getSecureRandom()->getLowStrengthGenerator()->generate(20));
+		}
+		return self::$reqId;
+	}
+
+	/**
 	 * Check overwrite condition
 	 * @param string $type
 	 * @return bool
diff --git a/lib/private/template.php b/lib/private/template.php
index d95943a714c..fce26117ede 100644
--- a/lib/private/template.php
+++ b/lib/private/template.php
@@ -250,8 +250,7 @@ class OC_Template extends \OC\Template\Base {
 	/**
 		* Print a fatal error page and terminates the script
 		* @param string $error_msg The error message to show
-		* @param string $hint An optional hint message
-		* Warning: All data passed to $hint needs to get sanitized using OC_Util::sanitizeHTML
+		* @param string $hint An optional hint message - needs to be properly escaped
 		*/
 	public static function printErrorPage( $error_msg, $hint = '' ) {
 		$content = new \OC_Template( '', 'error', 'error', false );
@@ -266,28 +265,16 @@ class OC_Template extends \OC\Template\Base {
 	 * @param Exception $exception
 	 */
 	public static function printExceptionErrorPage(Exception $exception) {
-		$error_msg = $exception->getMessage();
-		if ($exception->getCode()) {
-			$error_msg = '['.$exception->getCode().'] '.$error_msg;
-		}
-		if (defined('DEBUG') and DEBUG) {
-			$hint = $exception->getTraceAsString();
-			if (!empty($hint)) {
-				$hint = '<pre>'.OC_Util::sanitizeHTML($hint).'</pre>';
-			}
-			while (method_exists($exception, 'previous') && $exception = $exception->previous()) {
-				$error_msg .= '<br/>Caused by:' . ' ';
-				if ($exception->getCode()) {
-					$error_msg .= '['.OC_Util::sanitizeHTML($exception->getCode()).'] ';
-				}
-				$error_msg .= OC_Util::sanitizeHTML($exception->getMessage());
-			};
-		} else {
-			$hint = '';
-			if ($exception instanceof \OC\HintException) {
-				$hint = OC_Util::sanitizeHTML($exception->getHint());
-			}
-		}
-		self::printErrorPage($error_msg, $hint);
+		$content = new \OC_Template('', 'exception', 'error', false);
+		$content->assign('errorMsg', $exception->getMessage());
+		$content->assign('errorCode', $exception->getCode());
+		$content->assign('file', $exception->getFile());
+		$content->assign('line', $exception->getLine());
+		$content->assign('trace', $exception->getTraceAsString());
+		$content->assign('debugMode', defined('DEBUG') && DEBUG === true);
+		$content->assign('remoteAddr', OC_Request::getRemoteAddress());
+		$content->assign('requestID', OC_Request::getRequestID());
+		$content->printPage();
+		die();
 	}
 }