From b45af1125c9eb4c2ce28a43974a67a563e0ca444 Mon Sep 17 00:00:00 2001 From: Tim Starling Date: Wed, 4 Feb 2009 09:10:32 +0000 Subject: [PATCH] Added basic support for Wietse Venema's taint feature. Fixed a few instances of shoddy code that it turned up, no actual vulnerabilities yet. --- includes/GlobalFunctions.php | 13 +++++++++++++ includes/User.php | 2 +- includes/UserMailer.php | 4 ++-- includes/WebRequest.php | 3 ++- includes/db/Database.php | 4 ++++ includes/parser/Preprocessor_DOM.php | 4 +++- includes/parser/Preprocessor_Hash.php | 4 +++- includes/templates/NoLocalSettings.php | 13 +++++-------- 8 files changed, 33 insertions(+), 14 deletions(-) diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php index 8af83aceb4..42e1e46fb9 100644 --- a/includes/GlobalFunctions.php +++ b/includes/GlobalFunctions.php @@ -89,6 +89,19 @@ if ( !function_exists( 'array_diff_key' ) ) { } } +// Support for Wietse Venema's taint feature +if ( !function_exists( 'istainted' ) ) { + function istainted( $var ) { + return 0; + } + function taint( $var, $level = 0 ) {} + function untaint( $var, $level = 0 ) {} + define( 'TC_HTML', 1 ); + define( 'TC_SHELL', 1 ); + define( 'TC_MYSQL', 1 ); + define( 'TC_PCRE', 1 ); + define( 'TC_SELF', 1 ); +} /// @endcond diff --git a/includes/User.php b/includes/User.php index eb8c5fe590..e0289f8b95 100644 --- a/includes/User.php +++ b/includes/User.php @@ -915,7 +915,7 @@ class User { $this->mDataLoaded = true; if ( isset( $row->user_id ) ) { - $this->mId = $row->user_id; + $this->mId = intval( $row->user_id ); } $this->mName = $row->user_name; $this->mRealName = $row->user_real_name; diff --git a/includes/UserMailer.php b/includes/UserMailer.php index a2d4baef0a..7728abf329 100644 --- a/includes/UserMailer.php +++ b/includes/UserMailer.php @@ -363,7 +363,7 @@ class EmailNotification { if ( $wgEnotifWatchlist ) { // Send updates to watchers other than the current editor - $userCondition = 'wl_user != ' . $editor->getID(); + $userCondition = 'wl_user != ' . intval( $editor->getID() ); if ( $userTalkId !== false ) { // Already sent an email to this person $userCondition .= ' AND wl_user != ' . intval( $userTalkId ); @@ -415,7 +415,7 @@ class EmailNotification { 'wl_title' => $title->getDBkey(), 'wl_namespace' => $title->getNamespace(), 'wl_notificationtimestamp IS NULL', // store oldest unseen change time - 'wl_user != ' . $editor->getID() + 'wl_user != ' . intval( $editor->getID() ) ), __METHOD__ ); $dbw->commit(); diff --git a/includes/WebRequest.php b/includes/WebRequest.php index 467471250d..0928e4d51d 100644 --- a/includes/WebRequest.php +++ b/includes/WebRequest.php @@ -231,6 +231,7 @@ class WebRequest { $data = $this->normalizeUnicode( $data ); return $data; } else { + taint( $default ); return $default; } } @@ -251,7 +252,7 @@ class WebRequest { $val = $default; } if( is_null( $val ) ) { - return null; + return $val; } else { return (string)$val; } diff --git a/includes/db/Database.php b/includes/db/Database.php index 82db7da07b..38399186c9 100644 --- a/includes/db/Database.php +++ b/includes/db/Database.php @@ -566,6 +566,10 @@ class Database { } } + if ( istainted( $sql ) & TC_MYSQL ) { + throw new MWException( 'Tainted query found' ); + } + # Do the query and handle errors $ret = $this->doQuery( $commentedSql ); diff --git a/includes/parser/Preprocessor_DOM.php b/includes/parser/Preprocessor_DOM.php index af591b67d2..502d81f946 100644 --- a/includes/parser/Preprocessor_DOM.php +++ b/includes/parser/Preprocessor_DOM.php @@ -304,7 +304,9 @@ class Preprocessor_DOM implements Preprocessor { } else { $attrEnd = $tagEndPos; // Find closing tag - if ( preg_match( "/<\/$name\s*>/i", $text, $matches, PREG_OFFSET_CAPTURE, $tagEndPos + 1 ) ) { + if ( preg_match( "/<\/" . preg_quote( $name, '/' ) . "\s*>/i", + $text, $matches, PREG_OFFSET_CAPTURE, $tagEndPos + 1 ) ) + { $inner = substr( $text, $tagEndPos + 1, $matches[0][1] - $tagEndPos - 1 ); $i = $matches[0][1] + strlen( $matches[0][0] ); $close = '' . htmlspecialchars( $matches[0][0] ) . ''; diff --git a/includes/parser/Preprocessor_Hash.php b/includes/parser/Preprocessor_Hash.php index 620282914d..0e0fb57926 100644 --- a/includes/parser/Preprocessor_Hash.php +++ b/includes/parser/Preprocessor_Hash.php @@ -288,7 +288,9 @@ class Preprocessor_Hash implements Preprocessor { } else { $attrEnd = $tagEndPos; // Find closing tag - if ( preg_match( "/<\/$name\s*>/i", $text, $matches, PREG_OFFSET_CAPTURE, $tagEndPos + 1 ) ) { + if ( preg_match( "/<\/" . preg_quote( $name, '/' ) . "\s*>/i", + $text, $matches, PREG_OFFSET_CAPTURE, $tagEndPos + 1 ) ) + { $inner = substr( $text, $tagEndPos + 1, $matches[0][1] - $tagEndPos - 1 ); $i = $matches[0][1] + strlen( $matches[0][0] ); $close = $matches[0][0]; diff --git a/includes/templates/NoLocalSettings.php b/includes/templates/NoLocalSettings.php index 107f792ffd..6a566146de 100644 --- a/includes/templates/NoLocalSettings.php +++ b/includes/templates/NoLocalSettings.php @@ -4,10 +4,7 @@ * @ingroup Templates */ -# Prevent XSS -if ( isset( $wgVersion ) ) { - $wgVersion = htmlspecialchars( $wgVersion ); -} else { +if ( !isset( $wgVersion ) ) { $wgVersion = 'VERSION'; } @@ -40,7 +37,7 @@ foreach( $topdirs as $dir ){ - MediaWiki <?php echo $wgVersion ?> + MediaWiki <?php echo htmlspecialchars( $wgVersion ) ?> - The MediaWiki logo + The MediaWiki logo -

MediaWiki

+

MediaWiki

config/LocalSettings.php to the parent directory.' ); } else { - echo( "Please set up the wiki first." ); + echo( "Please set up the wiki first." ); } ?> -- 2.20.1