From 4ca98057ca42a1bd632aa5c879563d9a619d8427 Mon Sep 17 00:00:00 2001 From: Kevin Israel Date: Sun, 3 Mar 2013 09:27:47 -0500 Subject: [PATCH] Hide server IP addresses from DB error pages Error details are now omitted if both $wgShowHostnames and $wgShowSQLErrors are false. WMF wikis have the former set to true; users of those wikis can still include the details in bug reports. Fixing this for DBConnectionError was more or less straightforward. However, in the case of DBQueryError, this involved splitting the error message into multiple smaller messages (killing a raw HTML message in the process). Note that this is an incomplete fix: information disclosure is, for now, still possible from DBUnexpectedError exceptions (are these even supposed to go into the exception log?) or exceptions that occur from within the exception handler. Yet this is still an improvement. Bug: 26811 Change-Id: I1756b296d5e8d1d22511a3c3b58b5bb0dd025fec --- RELEASE-NOTES-1.22 | 2 + includes/db/DatabaseError.php | 194 +++++++++++++++++++---------- languages/messages/MessagesEn.php | 19 ++- languages/messages/MessagesQqq.php | 16 ++- maintenance/language/messages.inc | 9 +- 5 files changed, 159 insertions(+), 81 deletions(-) diff --git a/RELEASE-NOTES-1.22 b/RELEASE-NOTES-1.22 index b88e323dd4..859d6c9b52 100644 --- a/RELEASE-NOTES-1.22 +++ b/RELEASE-NOTES-1.22 @@ -285,6 +285,8 @@ production. can no longer be uploaded as an extension that we do know the mime type for. * (bug 51742) Add data-sort-value for better sorting of hitcounts Special:Tags +* (bug 26811) On DB error pages, server hostnames are now hidden when both + $wgShowHostnames and $wgShowSQLErrors are false. === API changes in 1.22 === * (bug 25553) The JSON output formatter now leaves forward slashes unescaped diff --git a/includes/db/DatabaseError.php b/includes/db/DatabaseError.php index 55095c3829..da391d7aac 100644 --- a/includes/db/DatabaseError.php +++ b/includes/db/DatabaseError.php @@ -42,25 +42,13 @@ class DBError extends MWException { parent::__construct( $error ); } - /** - * @param $html string - * @return string - */ - protected function getContentMessage( $html ) { - if ( $html ) { - return nl2br( htmlspecialchars( $this->getMessage() ) ); - } else { - return $this->getMessage(); - } - } - /** * @return string */ function getText() { global $wgShowDBErrorBacktrace; - $s = $this->getContentMessage( false ) . "\n"; + $s = $this->getTextContent() . "\n"; if ( $wgShowDBErrorBacktrace ) { $s .= "Backtrace:\n" . $this->getTraceAsString() . "\n"; @@ -75,14 +63,29 @@ class DBError extends MWException { function getHTML() { global $wgShowDBErrorBacktrace; - $s = $this->getContentMessage( true ); + $s = $this->getHTMLContent(); if ( $wgShowDBErrorBacktrace ) { - $s .= '

Backtrace:

' . nl2br( htmlspecialchars( $this->getTraceAsString() ) ); + $s .= '

Backtrace:

' . + nl2br( htmlspecialchars( $this->getTraceAsString() ) ) . '

'; } return $s; } + + /** + * @return string + */ + protected function getTextContent() { + return $this->getMessage(); + } + + /** + * @return string + */ + protected function getHTMLContent() { + return '

' . nl2br( htmlspecialchars( $this->getMessage() ) ) . '

'; + } } /** @@ -96,11 +99,12 @@ class DBConnectionError extends DBError { if ( trim( $error ) != '' ) { $msg .= ": $error"; + } elseif ( $db ) { + $error = $this->db->getServer(); } - $this->error = $error; - parent::__construct( $db, $msg ); + $this->error = $error; } /** @@ -141,39 +145,51 @@ class DBConnectionError extends DBError { * @return string */ function getPageTitle() { - global $wgSitename; - return htmlspecialchars( $this->msg( 'dberr-header', "$wgSitename has a problem" ) ); + return $this->msg( 'dberr-header', 'This wiki has a problem' ); } /** * @return string */ function getHTML() { - global $wgShowDBErrorBacktrace; + global $wgShowDBErrorBacktrace, $wgShowHostnames, $wgShowSQLErrors; - $sorry = htmlspecialchars( $this->msg( 'dberr-problems', 'Sorry! This site is experiencing technical difficulties.' ) ); + $sorry = htmlspecialchars( $this->msg( 'dberr-problems', "Sorry!\nThis site is experiencing technical difficulties." ) ); $again = htmlspecialchars( $this->msg( 'dberr-again', 'Try waiting a few minutes and reloading.' ) ); - $info = htmlspecialchars( $this->msg( 'dberr-info', '(Can\'t contact the database server: $1)' ) ); - # No database access - MessageCache::singleton()->disable(); - - if ( trim( $this->error ) == '' && $this->db ) { - $this->error = $this->db->getProperty( 'mServer' ); + if ( $wgShowHostnames || $wgShowSQLErrors ) { + $info = str_replace( + '$1', Html::element( 'span', array( 'dir' => 'ltr' ), $this->error ), + htmlspecialchars( $this->msg( 'dberr-info', '(Cannot contact the database server: $1)' ) ) + ); + } else { + $info = htmlspecialchars( $this->msg( 'dberr-info-hidden', '(Cannot contact the database server)' ) ); } - $this->error = Html::element( 'span', array( 'dir' => 'ltr' ), $this->error ); + # No database access + MessageCache::singleton()->disable(); - $noconnect = "

$sorry

$again

$info

"; - $text = str_replace( '$1', $this->error, $noconnect ); + $text = "

$sorry

$again

$info

"; if ( $wgShowDBErrorBacktrace ) { - $text .= '

Backtrace:

' . nl2br( htmlspecialchars( $this->getTraceAsString() ) ); + $text .= '

Backtrace:

' . + nl2br( htmlspecialchars( $this->getTraceAsString() ) ) . '

'; } - $extra = $this->searchForm(); + $text .= '
'; + $text .= $this->searchForm(); - return "$text
$extra"; + return $text; + } + + protected function getTextContent() { + global $wgShowHostnames, $wgShowSQLErrors; + + if ( $wgShowHostnames || $wgShowSQLErrors ) { + return $this->getMessage(); + } else { + return 'DB connection error'; + } } public function reportHTML() { @@ -302,55 +318,107 @@ class DBQueryError extends DBError { } /** - * @param $html string + * @return bool + */ + function getLogMessage() { + # Don't send to the exception log + return false; + } + + /** + * @return String + */ + function getPageTitle() { + return $this->msg( 'databaseerror', 'Database error' ); + } + + /** * @return string */ - function getContentMessage( $html ) { - if ( $this->useMessageCache() ) { - if ( $html ) { - $msg = 'dberrortext'; - $sql = htmlspecialchars( $this->getSQL() ); - $fname = htmlspecialchars( $this->fname ); - $error = htmlspecialchars( $this->error ); - } else { - $msg = 'dberrortextcl'; - $sql = $this->getSQL(); - $fname = $this->fname; - $error = $this->error; + protected function getHTMLContent() { + $key = 'databaseerror-text'; + $s = Html::element( 'p', array(), $this->msg( $key, $this->getFallbackMessage( $key ) ) ); + + $details = $this->getTechnicalDetails(); + if ( $details ) { + $s .= ''; } + + return $s; } /** - * @return String + * @return string */ - function getSQL() { - global $wgShowSQLErrors; + protected function getTextContent() { + $key = 'databaseerror-textcl'; + $s = $this->msg( $key, $this->getFallbackMessage( $key ) ) . "\n"; - if ( !$wgShowSQLErrors ) { - return $this->msg( 'sqlhidden', 'SQL hidden' ); - } else { - return $this->sql; + foreach ( $this->getTechnicalDetails() as $key => $detail ) { + $s .= $this->msg( $key, $this->getFallbackMessage( $key ), $detail[2] ) . "\n"; } + + return $s; } /** - * @return bool + * Make a list of technical details that can be shown to the user. This information can + * aid in debugging yet may be useful to an attacker trying to exploit a security weakness + * in the software or server configuration. + * + * Thus no such details are shown by default, though if $wgShowHostnames is true, only the + * full SQL query is hidden; in fact, the error message often does contain a hostname, and + * sites using this option probably don't care much about "security by obscurity". Of course, + * if $wgShowSQLErrors is true, the SQL query *is* shown. + * + * @return array: Keys are message keys; values are arrays of arguments for Html::element(). + * Array will be empty if users are not allowed to see any of these details at all. */ - function getLogMessage() { - # Don't send to the exception log - return false; + protected function getTechnicalDetails() { + global $wgShowHostnames, $wgShowSQLErrors; + + $attribs = array( 'dir' => 'ltr' ); + $details = array(); + + if ( $wgShowSQLErrors ) { + $details['databaseerror-query'] = array( + 'div', array( 'class' => 'mw-code' ) + $attribs, $this->sql ); + } + + if ( $wgShowHostnames || $wgShowSQLErrors ) { + $errorMessage = $this->errno . ' ' . $this->error; + $details['databaseerror-function'] = array( 'code', $attribs, $this->fname ); + $details['databaseerror-error'] = array( 'samp', $attribs, $errorMessage ); + } + + return $details; } /** - * @return String + * @param string $key Message key + * @return string: English message text */ - function getPageTitle() { - return $this->msg( 'databaseerror', 'Database error' ); + private function getFallbackMessage( $key ) { + $messages = array( + 'databaseerror-text' => 'A database query error has occurred. +This may indicate a bug in the software.', + 'databaseerror-textcl' => 'A database query error has occurred.', + 'databaseerror-query' => 'Query: $1', + 'databaseerror-function' => 'Function: $1', + 'databaseerror-error' => 'Error: $1', + ); + return $messages[$key]; } + } /** diff --git a/languages/messages/MessagesEn.php b/languages/messages/MessagesEn.php index 919a6ed8ff..23adfbff5c 100644 --- a/languages/messages/MessagesEn.php +++ b/languages/messages/MessagesEn.php @@ -1002,17 +1002,12 @@ A list of valid special pages can be found at [[Special:SpecialPages|{{int:speci # General errors 'error' => 'Error', 'databaseerror' => 'Database error', -'dberrortext' => 'A database query syntax error has occurred. -This may indicate a bug in the software. -The last attempted database query was: -
$1
-from within function "$2". -Database returned error "$3: $4".', -'dberrortextcl' => 'A database query syntax error has occurred. -The last attempted database query was: -"$1" -from within function "$2". -Database returned error "$3: $4"', +'databaseerror-text' => 'A database query error has occurred. +This may indicate a bug in the software.', +'databaseerror-textcl' => 'A database query error has occurred.', +'databaseerror-query' => 'Query: $1', +'databaseerror-function' => 'Function: $1', +'databaseerror-error' => 'Error: $1', 'laggedslavemode' => "'''Warning:''' Page may not contain recent updates.", 'readonly' => 'Database locked', 'enterlockreason' => 'Enter a reason for the lock, including an estimate of when the lock will be released', @@ -1070,7 +1065,6 @@ To add or change translations for all wikis, please use [//translatewiki.net/ tr 'editinginterface' => "'''Warning:''' You are editing a page that is used to provide interface text for the software. Changes to this page will affect the appearance of the user interface for other users on this wiki. To add or change translations for all wikis, please use [//translatewiki.net/ translatewiki.net], the MediaWiki localisation project.", -'sqlhidden' => '(SQL query hidden)', 'cascadeprotected' => 'This page has been protected from editing because it is included in the following {{PLURAL:$1|page, which is|pages, which are}} protected with the "cascading" option turned on: $2', 'namespaceprotected' => "You do not have permission to edit pages in the '''$1''' namespace.", @@ -4965,6 +4959,7 @@ You should have received [{{SERVER}}{{SCRIPTPATH}}/COPYING a copy of the GNU Gen This site is experiencing technical difficulties.', 'dberr-again' => 'Try waiting a few minutes and reloading.', 'dberr-info' => '(Cannot contact the database server: $1)', +'dberr-info-hidden' => '(Cannot contact the database server)', 'dberr-usegoogle' => 'You can try searching via Google in the meantime.', 'dberr-outofdate' => 'Note that their indexes of our content may be out of date.', 'dberr-cachederror' => 'This is a cached copy of the requested page, and may not be up to date.', diff --git a/languages/messages/MessagesQqq.php b/languages/messages/MessagesQqq.php index 165c552c35..b5d30496a5 100644 --- a/languages/messages/MessagesQqq.php +++ b/languages/messages/MessagesQqq.php @@ -1024,9 +1024,7 @@ This error is shown when trying to open a special page which does not exist, e.g # General errors 'error' => '{{Identical|Error}}', -'databaseerror' => 'Used as title of error message (one of the following messages): -* {{msg-mw|Dberrortext}} -* {{msg-mw|Dberrortextcl}}', +'databaseerror' => 'Used as title of error message {{msg-mw|Databaseerrortext}}.', 'dberrortext' => 'Parameters: * $1 - The last SQL command/query * $2 - SQL function name @@ -1037,6 +1035,17 @@ This error is shown when trying to open a special page which does not exist, e.g * $2 - SQL function name * $3 - Error number * $4 - Error description', +'databaseerror-text' => 'A list of technical details might (or might not) follow, depending on server configuration. Do not use wiki markup.', +'databaseerror-textcl' => 'Abridged form of {{msg-mw|databaseerror-text}}, suitable for use in command-line scripts run by the server administrator. Do not use wiki markup.', +'databaseerror-query' => 'Identifies, in the list of technical details, the [[wikipedia:SQL|SQL]] statement that failed. +Parameters: +* $1 - SQL statement (shown within a box)', +'databaseerror-function' => 'Identifies, in the list of technical details, the function that tried to execute the database query. +Parameters: +* $1 - Name of function', +'databaseerror-error' => 'Identifies, in the list of technical details, the error message the database server returned. +Parameters: +* $1 - Error message from the database server, probably in English', 'laggedslavemode' => 'Used as warning when getting the timestamp of the latest version, if in LaggedSlaveMode.', 'readonly' => 'Used as title of error message when database is locked.', 'enterlockreason' => 'For developers when locking the database', @@ -9778,6 +9787,7 @@ Parameters: 'dberr-again' => 'This message does not allow any wiki nor html markup.', 'dberr-info' => 'This message does not allow any wiki nor html markup. * $1 - database server name', +'dberr-info-hidden' => 'This message does not allow any wiki nor html markup.', 'dberr-usegoogle' => 'This message does not allow any wiki nor html markup.', 'dberr-outofdate' => "{{doc-singularthey}} In this sentence, '''their''' indexes refers to '''Google's''' indexes. This message does not allow any wiki nor html markup.", diff --git a/maintenance/language/messages.inc b/maintenance/language/messages.inc index d93960f337..f23f08c1d8 100644 --- a/maintenance/language/messages.inc +++ b/maintenance/language/messages.inc @@ -377,8 +377,11 @@ $wgMessageStructure = array( 'errors' => array( 'error', 'databaseerror', - 'dberrortext', - 'dberrortextcl', + 'databaseerror-text', + 'databaseerror-textcl', + 'databaseerror-query', + 'databaseerror-function', + 'databaseerror-error', 'laggedslavemode', 'readonly', 'enterlockreason', @@ -419,7 +422,6 @@ $wgMessageStructure = array( 'viewyourtext', 'protectedinterface', 'editinginterface', - 'sqlhidden', 'cascadeprotected', 'namespaceprotected', 'customcssprotected', @@ -3796,6 +3798,7 @@ $wgMessageStructure = array( 'dberr-problems', 'dberr-again', 'dberr-info', + 'dberr-info-hidden', 'dberr-usegoogle', 'dberr-outofdate', 'dberr-cachederror', -- 2.20.1