From 17e7bc02357e42a78cf5fdcbf9e550dda4631ac6 Mon Sep 17 00:00:00 2001
From: Brian Wolff <bawolff+wn@gmail.com>
Date: Thu, 10 Mar 2016 20:08:06 -0500
Subject: [PATCH] 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
---
 RELEASE-NOTES-1.29               |  2 ++
 includes/parser/Parser.php       | 11 ++++-------
 includes/parser/ParserOutput.php |  4 ++++
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/RELEASE-NOTES-1.29 b/RELEASE-NOTES-1.29
index 8b099bd893..b835eb5470 100644
--- 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 be4557d6f9..953f021c2b 100644
--- 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 b2f99b3d3f..7de3b304f1 100644
--- 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 );
-- 
2.20.1