* (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 33368de..34923fa 100644 (file)
--- 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 ce41c00..20f9fbb 100644 (file)
--- 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 9f05ed8..cac176b 100644 (file)
--- 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( '>', '>', $rest );
                                        $text .= "<$slash$t$newparams$brace$rest";

diff --git a/maintenance/parserTests.txt b/maintenance/parserTests.txt
index 94b965b..c3d7002 100644 (file)
--- 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