From 3ce2497720270d956f2ff27356ff88a24de16e62 Mon Sep 17 00:00:00 2001
From: Arlo Breault <abreault@wikimedia.org>
Date: Thu, 29 Jun 2023 17:03:15 -0400
Subject: [PATCH] SECURITY: Move badFile lookup to Linker

Bug: T335612
Change-Id: I849d02f1d3dc9995353b7a9995601d214053dca3
---
 includes/Linker.php        | 18 ++++++++++++++----
 includes/parser/Parser.php | 34 ++++++++++++++++------------------
 2 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/includes/Linker.php b/includes/Linker.php
index 39f15ec3104..f81c4ab6be2 100644
--- a/includes/Linker.php
+++ b/includes/Linker.php
@@ -392,7 +392,7 @@ class Linker {
 				$frameParams['align'] = $parser->getTargetLanguage()->alignEnd();
 			}
 			return $prefix .
-				self::makeThumbLink2( $title, $file, $frameParams, $handlerParams, $time, $query ) .
+				self::makeThumbLink2( $title, $file, $frameParams, $handlerParams, $time, $query, $parser ) .
 				$postfix;
 		}
 
@@ -413,7 +413,10 @@ class Linker {
 			$thumb = false;
 		}
 
-		if ( !$thumb ) {
+		$isBadFile = $file && $thumb && $parser &&
+			$parser->getBadFileLookup()->isBadFile( $title->getDBkey(), $parser->getTitle() );
+
+		if ( !$thumb || $isBadFile ) {
 			$s = self::makeBrokenImageLinkObj( $title, $frameParams['title'], '', '', '', $time == true );
 		} else {
 			self::processResponsiveImages( $file, $thumb, $handlerParams );
@@ -511,10 +514,11 @@ class Linker {
 	 * @param array $handlerParams
 	 * @param bool $time
 	 * @param string $query
+	 * @param Parser|null $parser
 	 * @return string
 	 */
 	public static function makeThumbLink2( LinkTarget $title, $file, $frameParams = [],
-		$handlerParams = [], $time = false, $query = ""
+		$handlerParams = [], $time = false, $query = "", ?Parser $parser = null
 	) {
 		$exists = $file && $file->exists();
 
@@ -594,10 +598,16 @@ class Linker {
 		$s = "<div class=\"thumb t{$frameParams['align']}\">"
 			. "<div class=\"thumbinner\" style=\"width:{$outerWidth}px;\">";
 
+		$isBadFile = $exists && $thumb && $parser &&
+			$parser->getBadFileLookup()->isBadFile(
+				$manualthumb ? $manual_title : $title->getDBkey(),
+				$parser->getTitle()
+			);
+
 		if ( !$exists ) {
 			$s .= self::makeBrokenImageLinkObj( $title, $frameParams['title'], '', '', '', $time == true );
 			$zoomIcon = '';
-		} elseif ( !$thumb ) {
+		} elseif ( !$thumb || $isBadFile ) {
 			$s .= wfMessage( 'thumbnail_error', '' )->escaped();
 			$zoomIcon = '';
 		} else {
diff --git a/includes/parser/Parser.php b/includes/parser/Parser.php
index 1295165d0b9..4a1615f9007 100644
--- a/includes/parser/Parser.php
+++ b/includes/parser/Parser.php
@@ -2616,25 +2616,23 @@ class Parser {
 				}
 
 				if ( $ns == NS_FILE ) {
-					if ( !$this->badFileLookup->isBadFile( $nt->getDBkey(), $this->getTitle() ) ) {
-						if ( $wasblank ) {
-							# if no parameters were passed, $text
-							# becomes something like "File:Foo.png",
-							# which we don't want to pass on to the
-							# image generator
-							$text = '';
-						} else {
-							# recursively parse links inside the image caption
-							# actually, this will parse them in any other parameters, too,
-							# but it might be hard to fix that, and it doesn't matter ATM
-							$text = $this->handleExternalLinks( $text );
-							$holders->merge( $this->handleInternalLinks2( $text ) );
-						}
-						# cloak any absolute URLs inside the image markup, so handleExternalLinks() won't touch them
-						$s .= $prefix . $this->armorLinks(
-							$this->makeImage( $nt, $text, $holders ) ) . $trail;
-						continue;
+					if ( $wasblank ) {
+						# if no parameters were passed, $text
+						# becomes something like "File:Foo.png",
+						# which we don't want to pass on to the
+						# image generator
+						$text = '';
+					} else {
+						# recursively parse links inside the image caption
+						# actually, this will parse them in any other parameters, too,
+						# but it might be hard to fix that, and it doesn't matter ATM
+						$text = $this->handleExternalLinks( $text );
+						$holders->merge( $this->handleInternalLinks2( $text ) );
 					}
+					# cloak any absolute URLs inside the image markup, so handleExternalLinks() won't touch them
+					$s .= $prefix . $this->armorLinks(
+						$this->makeImage( $nt, $text, $holders ) ) . $trail;
+					continue;
 				} elseif ( $ns == NS_CATEGORY ) {
 					/**
 					 * Strip the whitespace Category links produce, see T2087
-- 
2.34.1