From: Brion Vibber <brion@users.mediawiki.org>
Date: Mon, 6 Jun 2005 01:46:03 +0000 (+0000)
Subject: * (bug 2309) Allow templates and template parameters in HTML attribute zone,
X-Git-Tag: 1.5.0beta1~214
X-Git-Url: http://git.cyclocoop.org/%28%5B%5E//%22%22?a=commitdiff_plain;h=b42e3374632053388a8a9d095751f0936630c384;p=lhc%2Fweb%2Fwiklou.git

* (bug 2309) Allow templates and template parameters in HTML attribute zone,
  with proper validation checks. (regression from fix for 2304)
---

diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index 33368dea72..34923fae0f 100644
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -269,6 +269,8 @@ Various bugfixes, small features, and a few experimental things:
 * (bug 2223) Add unique index on user_name field to prevent duplicate accounts
 * (bug 1976) fix shared user database with a table prefix set
 * (bug 2334) Accept null for attribs in wfElement without PHP warning
+* (bug 2309) Allow templates and template parameters in HTML attribute zone,
+  with proper validation checks. (regression from fix for 2304)
 
 
 === Caveats ===
diff --git a/includes/Parser.php b/includes/Parser.php
index ce41c00876..20f9fbb555 100644
--- a/includes/Parser.php
+++ b/includes/Parser.php
@@ -278,7 +278,7 @@ class Parser
 			$start = '/<!--()/';
 			$end   = '/-->/';
 		} else {
-			$start = "/<$tag(\\s+[^>]*|\\s*)>/i";
+			$start = "/<$tag([^>]*)>/i";
 			$end   = "/<\\/$tag\\s*>/i";
 		}
 
@@ -753,7 +753,7 @@ class Parser
 		$fname = 'Parser::internalParse';
 		wfProfileIn( $fname );
 
-		$text = Sanitizer::removeHTMLtags( $text );
+		$text = Sanitizer::removeHTMLtags( $text, array( &$this, 'replaceVariables' ) );
 		$text = $this->replaceVariables( $text, $args );
 
 		$text = preg_replace( '/(^|\n)-----*/', '\\1<hr />', $text );
@@ -2252,7 +2252,7 @@ class Parser
 
 			if( $this->mOutputType == OT_HTML ) {
 				$text = $this->strip( $text, $this->mStripState );
-				$text = Sanitizer::removeHTMLtags( $text );
+				$text = Sanitizer::removeHTMLtags( $text, array( &$this, 'replaceVariables' ), $assocArgs );
 			}
 			$text = $this->replaceVariables( $text, $assocArgs );
 
diff --git a/includes/Sanitizer.php b/includes/Sanitizer.php
index 9f05ed87cd..cac176bc1d 100644
--- a/includes/Sanitizer.php
+++ b/includes/Sanitizer.php
@@ -323,9 +323,11 @@ class Sanitizer {
 	 * removes HTML comments
 	 * @access private
 	 * @param string $text
+	 * @param callback $processCallback to do any variable or parameter replacements in HTML attribute values
+	 * @param array $args for the processing callback
 	 * @return string
 	 */
-	function removeHTMLtags( $text ) {
+	function removeHTMLtags( $text, $processCallback = null, $args = array() ) {
 		global $wgUseTidy, $wgUserHtml;
 		$fname = 'Parser::removeHTMLtags';
 		wfProfileIn( $fname );
@@ -402,6 +404,13 @@ class Sanitizer {
 							}
 							array_push( $tagstack, $t );
 						}
+
+						# Replace any variables or template parameters with
+						# plaintext results.
+						if( is_callable( $processCallback ) ) {
+							call_user_func_array( $processCallback, array( &$params, $args ) );
+						}
+
 						# Strip non-approved attributes from the tag
 						$newparams = Sanitizer::fixTagAttributes( $params, $t );
 					}
@@ -425,6 +434,9 @@ class Sanitizer {
 				$x, $regs );
 				@list( $qbar, $slash, $t, $params, $brace, $rest ) = $regs;
 				if ( in_array( $t = strtolower( $t ), $htmlelements ) ) {
+					if( is_callable( $processCallback ) ) {
+						call_user_func_array( $processCallback, array( &$params, $args ) );
+					}
 					$newparams = Sanitizer::fixTagAttributes( $params, $t );
 					$rest = str_replace( '>', '&gt;', $rest );
 					$text .= "<$slash$t$newparams$brace$rest";
diff --git a/maintenance/parserTests.txt b/maintenance/parserTests.txt
index 94b965b9bc..c3d700285b 100644
--- a/maintenance/parserTests.txt
+++ b/maintenance/parserTests.txt
@@ -2350,12 +2350,76 @@ Bug 2095: link with pipe and three closing brackets
 ### Safety
 ###
 
+!! article
+Template:Dangerous attribute
+!! text
+" onmouseover="alert(document.cookie)
+!! endarticle
+
+!! article
+Template:Dangerous style attribute
+!! text
+border-size: expression(alert(document.cookie))
+!! endarticle
+
+!! article
+Template:Div style
+!! text
+<div style="float: right; {{{1}}}">Magic div</div>
+!! endarticle
+
 !! test
-Bug 2304: HTML attribute safety (template)
+Bug 2304: HTML attribute safety (safe template; regression bug 2309)
 !! input
 <div title="{{test}}"></div>
 !! result
-<div title="&#123;&#123;test}}"></div>
+<div title="This is a test template"></div>
+
+!! end
+
+!! test
+Bug 2304: HTML attribute safety (dangerous template; 2309)
+!! input
+<div title="{{dangerous attribute}}"></div>
+!! result
+<div title=""></div>
+
+!! end
+
+!! test
+Bug 2304: HTML attribute safety (dangerous style template; 2309)
+!! input
+<div style="{{dangerous style attribute}}"></div>
+!! result
+<div></div>
+
+!! end
+
+!! test
+Bug 2304: HTML attribute safety (safe parameter; 2309)
+!! input
+{{div style|width: 200px}}
+!! result
+<div style="float: right; width: 200px">Magic div</div>
+
+!! end
+
+!! test
+Bug 2304: HTML attribute safety (unsafe parameter; 2309)
+!! input
+{{div style|width: expression(alert(document.cookie))}}
+!! result
+<div>Magic div</div>
+
+!! end
+
+
+!! test
+Bug 2304: HTML attribute safety (unsafe breakout parameter; 2309)
+!! input
+{{div style|"><script>alert(document.cookie)</script>}}
+!! result
+<div>Magic div</div>
 
 !! end