From: Brion Vibber <brion@users.mediawiki.org>
Date: Tue, 23 Aug 2005 21:49:48 +0000 (+0000)
Subject: * (bug 3244) Fix remote image loading hack, JavaScript injection on MSIE
X-Git-Tag: 1.6.0~1826
X-Git-Url: http://git.cyclocoop.org/%7B%24www_url%7Dadmin/membres/fiche.php?a=commitdiff_plain;h=1c21932d3755a7006e7a7ad2f647c39f92b8a573;p=lhc%2Fweb%2Fwiklou.git

* (bug 3244) Fix remote image loading hack, JavaScript injection on MSIE
---

diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index 5e36481c1a..c460f6d0e1 100644
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -49,6 +49,7 @@ Misc work going on.....
   so they get called on non-EditPage actions that use these functions
   to create or update pages.
 * Fix table prefix usage in Block::enumBlocks
+* (bug 3244) Fix remote image loading hack, JavaScript injection on MSIE
 
 
 === Caveats ===
diff --git a/includes/Parser.php b/includes/Parser.php
index 2aae7e6a08..3bc48ff1f4 100644
--- a/includes/Parser.php
+++ b/includes/Parser.php
@@ -765,7 +765,7 @@ class Parser
 		$text = strtr( $text, array( '<noinclude>' => '', '</noinclude>' => '') );
 		$text = preg_replace( '/<includeonly>.*?<\/includeonly>/s', '', $text );
 
-		$text = Sanitizer::removeHTMLtags( $text, array( &$this, 'replaceVariables' ) );
+		$text = Sanitizer::removeHTMLtags( $text, array( &$this, 'attributeStripCallback' ) );
 		$text = $this->replaceVariables( $text, $args );
 
 		$text = preg_replace( '/(^|\n)-----*/', '\\1<hr />', $text );
@@ -3296,6 +3296,21 @@ class Parser
 	function disableCache() {
 		$this->mOutput->mCacheTime = -1;
 	}
+	
+	/**
+	 * Callback from the Sanitizer for expanding items found in HTML attribute
+	 * values, so they can be safely tested and escaped.
+	 * @param string $text
+	 * @param array $args
+	 * @return string
+	 * @access private
+	 */
+	function attributeStripCallback( &$text, $args ) {
+		$text = $this->replaceVariables( $text, $args );
+		$text = $this->unstrip( $text, $this->mStripState );
+		$text = $this->unstripNoWiki( $text, $this->mStripState );
+		return $text;
+	}
 }
 
 /**
diff --git a/includes/Sanitizer.php b/includes/Sanitizer.php
index c2604bab4d..e0217ba5eb 100644
--- a/includes/Sanitizer.php
+++ b/includes/Sanitizer.php
@@ -571,9 +571,12 @@ class Sanitizer {
 				'RFC'  => '&#82;FC',
 				'PMID' => '&#80;MID',
 			) );
-			$value = preg_replace(
-				'/(' . $wgUrlProtocols . '):/',
-				'\\1&#58;', $value );
+			
+			# Stupid hack
+			$value = preg_replace_callback(
+				'/(' . $wgUrlProtocols . ')/',
+				array( 'Sanitizer', 'armorLinksCallback' ),
+				$value );
 			
 			// If this attribute was previously set, override it.
 			// Output should only have one attribute of each name.
@@ -586,6 +589,16 @@ class Sanitizer {
 		}
 	}
 	
+	/**
+	 * Regex replace callback for armoring links against further processing.
+	 * @param array $matches
+	 * @return string
+	 * @access private
+	 */
+	function armorLinksCallback( $matches ) {
+		return str_replace( ':', '&#58;', $matches[1] );
+	}
+	
 	/**
 	 * Return an associative array of attribute names and values from
 	 * a partial tag string. Attribute names are forces to lowercase,
diff --git a/maintenance/parserTests.txt b/maintenance/parserTests.txt
index cc04652b55..f3342af9a0 100644
--- a/maintenance/parserTests.txt
+++ b/maintenance/parserTests.txt
@@ -2579,6 +2579,24 @@ Bug 2304: HTML attribute safety (named web link)
 
 !! end
 
+!! test
+Bug 3244: HTML attribute safety (extension; safe)
+!! input
+<div style="<nowiki>background:blue</nowiki>"></div>
+!! result
+<div style="background:blue"></div>
+
+!! end
+
+!! test
+Bug 3244: HTML attribute safety (extension; unsafe)
+!! input
+<div style="<nowiki>border-left:expression(alert(document.cookie))</nowiki>"></div>
+!! result
+<div></div>
+
+!! end
+
 
 TODO:
 more images