Hide server IP addresses from DB error pages
authorKevin Israel <pleasestand@live.com>
Sun, 3 Mar 2013 14:27:47 +0000 (09:27 -0500)
committerKevin Israel <pleasestand@live.com>
Tue, 3 Sep 2013 20:55:52 +0000 (16:55 -0400)
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
includes/db/DatabaseError.php
languages/messages/MessagesEn.php
languages/messages/MessagesQqq.php
maintenance/language/messages.inc

index b88e323..859d6c9 100644 (file)
@@ -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
index 55095c3..da391d7 100644 (file)
@@ -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 .= '<p>Backtrace:</p><p>' . nl2br( htmlspecialchars( $this->getTraceAsString() ) );
+                       $s .= '<p>Backtrace:</p><p>' .
+                               nl2br( htmlspecialchars( $this->getTraceAsString() ) ) . '</p>';
                }
 
                return $s;
        }
+
+       /**
+        * @return string
+        */
+       protected function getTextContent() {
+               return $this->getMessage();
+       }
+
+       /**
+        * @return string
+        */
+       protected function getHTMLContent() {
+               return '<p>' . nl2br( htmlspecialchars( $this->getMessage() ) ) . '</p>';
+       }
 }
 
 /**
@@ -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 = "<h1>$sorry</h1><p>$again</p><p><small>$info</small></p>";
-               $text = str_replace( '$1', $this->error, $noconnect );
+               $text = "<h1>$sorry</h1><p>$again</p><p><small>$info</small></p>";
 
                if ( $wgShowDBErrorBacktrace ) {
-                       $text .= '<p>Backtrace:</p><p>' . nl2br( htmlspecialchars( $this->getTraceAsString() ) );
+                       $text .= '<p>Backtrace:</p><p>' .
+                               nl2br( htmlspecialchars( $this->getTraceAsString() ) ) . '</p>';
                }
 
-               $extra = $this->searchForm();
+               $text .= '<hr />';
+               $text .= $this->searchForm();
 
-               return "$text<hr />$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 .= '<ul>';
+                       foreach ( $details as $key => $detail ) {
+                               $s .= str_replace(
+                                       '$1', call_user_func_array( 'Html::element', $detail ),
+                                       Html::element( 'li', array(),
+                                               $this->msg( $key, $this->getFallbackMessage( $key ) )
+                                       )
+                               );
                        }
-                       return wfMessage( $msg )->rawParams( $sql, $fname, $this->errno, $error )->text();
-               } else {
-                       return parent::getContentMessage( $html );
+                       $s .= '</ul>';
                }
+
+               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];
        }
+
 }
 
 /**
index 919a6ed..23adfbf 100644 (file)
@@ -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:
-<blockquote><code>$1</code></blockquote>
-from within function "<code>$2</code>".
-Database returned error "<samp>$3: $4</samp>".',
-'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.',
index 165c552..b5d3049 100644 (file)
@@ -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.",
index d93960f..f23f08c 100644 (file)
@@ -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',