Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F37123435
0001-SECURITY-Move-badFile-lookup-to-Linker.patch
Arlolra (Arlo Breault)
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Authored By
Arlolra
Jun 29 2023, 9:11 PM
2023-06-29 21:11:59 (UTC+0)
Size
4 KB
Referenced Files
None
Subscribers
None
0001-SECURITY-Move-badFile-lookup-to-Linker.patch
View Options
From 1094aed1d30e2d6a8c10241211565c6b68e554dd 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.41.0
File Metadata
Details
Attached
Mime Type
text/x-diff
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
10911979
Default Alt Text
0001-SECURITY-Move-badFile-lookup-to-Linker.patch (4 KB)
Attached To
Mode
T335612: CVE-2023-36674: Manualthumb bypasses badFile lookup
Attached
Detach File
Event Timeline
Log In to Comment