SECURITY: Always normalize link url before adding to ParserOutput

Move link normalization directly into addExternalLink() method,
since you always need to do it - having it separate is just
inviting people to forget to normalize a link.

Additionally, links weren't properly registered for <gallery>.
This was somewhat unnoticed, as the call to recursiveTagParse()
would register free links, but it wouldn't work for example with
protocol relative links.

Issue originally reported by MZMcBride.

Bug: T48143
Change-Id: I557fb3b433ef9d618097b6ba4eacc6bada250ca2

diff --git a/RELEASE-NOTES-1.29 b/RELEASE-NOTES-1.29
index 8b099bd..b835eb5 100644 (file)
--- a/RELEASE-NOTES-1.29
+++ b/RELEASE-NOTES-1.29
@@ -101,6 +101,8 @@ production.
   declaration.
 * (T161453) SECURITY: LocalisationCache will no longer use the temporary directory
   in it's fallback chain when trying to work out where to write the cache.
+* (T48143) SECURITY: Spam blacklist ineffective on encoded URLs inside file inclusion
+  syntax's link parameter.
 
 === Action API changes in 1.29 ===
 * Submitting sensitive authentication request parameters to action=login,

diff --git a/includes/parser/Parser.php b/includes/parser/Parser.php
index be4557d..953f021 100644 (file)
--- a/includes/parser/Parser.php
+++ b/includes/parser/Parser.php
@@ -1610,9 +1610,7 @@ class Parser {
                                true, 'free',
                                $this->getExternalLinkAttribs( $url ), $this->mTitle );
                        # Register it in the output object...
-                       # Replace unnecessary URL escape codes with their equivalent characters
-                       $pasteurized = self::normalizeLinkUrl( $url );
-                       $this->mOutput->addExternalLink( $pasteurized );
+                       $this->mOutput->addExternalLink( $url );
                }
                return $text . $trail;
        }
@@ -1908,10 +1906,7 @@ class Parser {
                                $this->getExternalLinkAttribs( $url ), $this->mTitle ) . $dtrail . $trail;
 
                        # Register link in the output object.
-                       # Replace unnecessary URL escape codes with the referenced character
-                       # This prevents spammers from hiding links from the filters
-                       $pasteurized = self::normalizeLinkUrl( $url );
-                       $this->mOutput->addExternalLink( $pasteurized );
+                       $this->mOutput->addExternalLink( $url );
                }
 
                return $s;
@@ -5086,9 +5081,11 @@ class Parser {
                                                        }
                                                        if ( preg_match( "/^($prots)$addr$chars*$/u", $linkValue ) ) {
                                                                $link = $linkValue;
+                                                               $this->mOutput->addExternalLink( $link );
                                                        } else {
                                                                $localLinkTitle = Title::newFromText( $linkValue );
                                                                if ( $localLinkTitle !== null ) {
+                                                                       $this->mOutput->addLink( $localLinkTitle );
                                                                        $link = $localLinkTitle->getLinkURL();
                                                                }
                                                        }

diff --git a/includes/parser/ParserOutput.php b/includes/parser/ParserOutput.php
index b2f99b3..7de3b30 100644 (file)
--- a/includes/parser/ParserOutput.php
+++ b/includes/parser/ParserOutput.php
@@ -535,6 +535,10 @@ class ParserOutput extends CacheTime {
                # We don't register links pointing to our own server, unless... :-)
                global $wgServer, $wgRegisterInternalExternals;
 
+               # Replace unnecessary URL escape codes with the referenced character
+               # This prevents spammers from hiding links from the filters
+               $url = parser::normalizeLinkUrl( $url );
+
                $registerExternalLink = true;
                if ( !$wgRegisterInternalExternals ) {
                        $registerExternalLink = !self::isLinkInternal( $wgServer, $url );