From: Alex Monk <krenair@gmail.com>
Date: Fri, 17 May 2013 20:27:18 +0000 (+0100)
Subject: Add a way to redact certain function parameters from exception stack traces
X-Git-Tag: 1.31.0-rc.0~18714
X-Git-Url: http://git.cyclocoop.org/%24href?a=commitdiff_plain;h=459cb0b7bb385d559c2a8cbf4e209a6795993794;p=lhc%2Fweb%2Fwiklou.git

Add a way to redact certain function parameters from exception stack traces

Bug: 30714
Change-Id: I0a9e92448f8d9009dd594cb4d7f5dc6fdd4bcc86
---

diff --git a/RELEASE-NOTES-1.22 b/RELEASE-NOTES-1.22
index 94a9dfdece..3ba86f533d 100644
--- a/RELEASE-NOTES-1.22
+++ b/RELEASE-NOTES-1.22
@@ -216,6 +216,8 @@ production.
 * Add deferrable update support for callback/closure
 * Add TitleMove hook before page renames
 * Revision deletion backend code is moved out of SpecialRevisiondelete
+* Add a variable (wgRedactedFunctionArguments) to redact the values sent as certain function
+  parameters from exception stack traces.
 
 === Bug fixes in 1.22 ===
 * Disable Special:PasswordReset when $wgEnableEmail is false. Previously one
diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 22b7f1ecd4..51c6c3d4a2 100644
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -4880,6 +4880,37 @@ $wgShowSQLErrors = false;
  */
 $wgShowExceptionDetails = false;
 
+/**
+ * Array of functions which need parameters redacted from stack traces shown to
+ * clients and logged. Keys are in the format '[class::]function', and the
+ * values should be either an integer or an array of integers. These are the
+ * indexes of the parameters which need to be kept secret.
+ * @since 1.22
+ */
+$wgRedactedFunctionArguments = array(
+	'AuthPlugin::setPassword' => 1,
+	'AuthPlugin::authenticate' => 1,
+	'AuthPlugin::addUser' => 1,
+
+	'DatabaseBase::__construct' => 2,
+	'DatabaseBase::open' => 2,
+
+	'SpecialChangeEmail::attemptChange' => 1,
+	'SpecialChangePassword::attemptReset' => 0,
+
+	'User::setPassword' => 0,
+	'User::setInternalPassword' => 0,
+	'User::checkPassword' => 0,
+	'User::setNewpassword' => 0,
+	'User::comparePasswords' => array( 0, 1 ),
+	'User::checkTemporaryPassword' => 0,
+	'User::setToken' => 0,
+	'User::crypt' => 0,
+	'User::oldCrypt' => 0,
+	'User::getPasswordValidity' => 0,
+	'User::isValidPassword' => 0,
+);
+
 /**
  * If true, show a backtrace for database errors
  */
diff --git a/includes/Exception.php b/includes/Exception.php
index e1bfb2dd56..39fe6f4b30 100644
--- a/includes/Exception.php
+++ b/includes/Exception.php
@@ -127,7 +127,7 @@ class MWException extends Exception {
 
 		if ( $wgShowExceptionDetails ) {
 			return '<p>' . nl2br( htmlspecialchars( $this->getMessage() ) ) .
-				'</p><p>Backtrace:</p><p>' . nl2br( htmlspecialchars( $this->getTraceAsString() ) ) .
+				'</p><p>Backtrace:</p><p>' . nl2br( htmlspecialchars( MWExceptionHandler::formatRedactedTrace( $this ) ) ) .
 				"</p>\n";
 		} else {
 			return "<div class=\"errorbox\">" .
@@ -152,7 +152,7 @@ class MWException extends Exception {
 
 		if ( $wgShowExceptionDetails ) {
 			return $this->getMessage() .
-				"\nBacktrace:\n" . $this->getTraceAsString() . "\n";
+				"\nBacktrace:\n" . MWExceptionHandler::formatRedactedTrace( $this ) . "\n";
 		} else {
 			return "Set \$wgShowExceptionDetails = true; " .
 				"in LocalSettings.php to show detailed debugging information.\n";
@@ -275,7 +275,7 @@ class MWException extends Exception {
 		$log = $this->getLogMessage();
 		if ( $log ) {
 			if ( $wgLogExceptionBacktrace ) {
-				wfDebugLog( 'exception', $log . "\n" . $this->getTraceAsString() . "\n" );
+				wfDebugLog( 'exception', $log . "\n" . MWExceptionHandler::formatRedactedTrace( $this ) . "\n" );
 			} else {
 				wfDebugLog( 'exception', $log );
 			}
@@ -633,7 +633,7 @@ class MWExceptionHandler {
 				$message = "MediaWiki internal error.\n\n";
 
 				if ( $wgShowExceptionDetails ) {
-					$message .= 'Original exception: ' . $e->__toString() . "\n\n" .
+					$message .= 'Original exception: ' . self::formatRedactedTrace( $e ) . "\n\n" .
 						'Exception caught inside exception handler: ' . $e2->__toString();
 				} else {
 					$message .= "Exception caught inside exception handler.\n\n" .
@@ -650,11 +650,10 @@ class MWExceptionHandler {
 				}
 			}
 		} else {
-			$message = "Unexpected non-MediaWiki exception encountered, of type \"" . get_class( $e ) . "\"\n" .
-				$e->__toString() . "\n";
+			$message = "Unexpected non-MediaWiki exception encountered, of type \"" . get_class( $e ) . "\"";
 
 			if ( $wgShowExceptionDetails ) {
-				$message .= "\n" . $e->getTraceAsString() . "\n";
+				$message .= "\nexception '" . get_class( $e ) . "' in " . $e->getFile() . ":" . $e->getLine() . "\nStack trace:\n" . self::formatRedactedTrace( $e ) . "\n";
 			}
 
 			if ( $cmdLine ) {
@@ -709,4 +708,53 @@ class MWExceptionHandler {
 		// Exit value should be nonzero for the benefit of shell jobs
 		exit( 1 );
 	}
+
+	/**
+	 * Get the stack trace from the exception as a string, redacting certain function arguments in the process
+	 * @param Exception $e The exception
+	 * @return string The stack trace as a string
+	 */
+	public static function formatRedactedTrace( Exception $e ) {
+		global $wgRedactedFunctionArguments;
+		$finalExceptionText = '';
+
+		foreach ( $e->getTrace() as $i => $call ) {
+			$checkFor = array();
+			if ( isset( $call['class'] ) ) {
+				$checkFor[] = $call['class'] . '::' . $call['function'];
+				foreach ( class_parents( $call['class'] ) as $parent ) {
+					$checkFor[] = $parent . '::' . $call['function'];
+				}
+			} else {
+				$checkFor[] = $call['function'];
+			}
+
+			foreach ( $checkFor as $check ) {
+				if ( isset( $wgRedactedFunctionArguments[$check] ) ) {
+					foreach ( (array)$wgRedactedFunctionArguments[$check] as $argNo ) {
+						$call['args'][$argNo] = 'REDACTED';
+					}
+				}
+			}
+
+			$finalExceptionText .= "#{$i} {$call['file']}({$call['line']}): ";
+			if ( isset( $call['class'] ) ) {
+				$finalExceptionText .= $call['class'] . $call['type'] . $call['function'];
+			} else {
+				$finalExceptionText .= $call['function'];
+			}
+			$args = array();
+			foreach ( $call['args'] as $arg ) {
+				if ( is_object( $arg ) ) {
+					$args[] = 'Object(' . get_class( $arg ) . ')';
+				} elseif( is_array( $arg ) ) {
+					$args[] = 'Array';
+				} else {
+					$args[] = var_export( $arg, true );
+				}
+			}
+			$finalExceptionText .=  '(' . implode( ', ', $args ) . ")\n";
+		}
+		return $finalExceptionText . '#' . ( $i + 1 ) . ' {main}';
+	}
 }