Page MenuHomePhabricator

CVE-2023-36674: Manualthumb bypasses badFile lookup
Closed, ResolvedPublicSecurity

Assigned To
Authored By
Arlolra
Apr 28 2023, 11:16 PM
Referenced Files
F37123459: 0001-SECURITY-Move-badFile-lookup-to-Linker-REL1_35v2.patch
Jun 29 2023, 10:00 PM
F37123452: 0001-SECURITY-Move-badFile-lookup-to-Linker-REL1_38.patch
Jun 29 2023, 9:52 PM
F37123435: 0001-SECURITY-Move-badFile-lookup-to-Linker.patch
Jun 29 2023, 9:22 PM
F37123316: 0001-SECURITY-Move-badFile-lookup-to-Linker.patch
Jun 29 2023, 6:41 PM
F37123301: 0001-SECURITY-Move-badFile-lookup-to-Linker.patch
Jun 29 2023, 6:08 PM
F37122589: 0005-SECURITY-Move-badFile-lookup-to-Linker.patch
Jun 29 2023, 1:01 AM
F37122590: 0003-Handle-thumb-errors-when-enableLegacyMediaDOM.patch
Jun 29 2023, 1:01 AM

Description

If "Bad.jpg" is listed on MediaWiki:Bad image list, you can still do [[File:Good.jpg|thumb=Bad.jpg|Uh oh]]

Here's a patch to fix it,

Unfortunately, it's sitting behind this chain I'm working on https://gerrit.wikimedia.org/r/q/topic:T334659


BranchPatch
REL1_35See T335612#8978250
REL1_38Omitted T335612#8977627
REL1_39
REL1_40
master

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
mmartorana changed the task status from Open to In Progress.May 5 2023, 3:33 PM
mmartorana triaged this task as Low priority.
mmartorana changed Risk Rating from N/A to Low.

Unfortunately, it's sitting behind this chain I'm working on https://gerrit.wikimedia.org/r/q/topic:T334659

These are all merged now.

The patch here will resolve T329214 as well.

Tested the patch & review C+2. Looks good, works in my testing for manual thumb and for ordinary image links, the exception mechanism in MediaWiki:bad_image_list works for manual thumb too.

sbassett subscribed.

Thanks for the CR, @cscott. And thanks, @Arlolra, for writing the patch. Looks like it applies to current master and 1.41.0-wmf.8 but not 1.41.0-wmf.7, the latter likely due to the gerrit relation chain issues discussed above. We can try to deploy this maybe sometime Thursday (5/11/2023), post-train, once everything is on 1.41.0-wmf.8 or possibly during next Monday's (5/15/2023) standard security deployment window, unless this is far more urgent.

unless this is far more urgent.

I think it can wait until one of those windows

Added [SECURITY] to the patch subject and renamed for production deployment:


Still tests fine on master and wmf.8. Should be tracked under T333617 and T276237 once deployed.

Added [SECURITY] to the patch subject and renamed for production deployment:


Still tests fine on master and wmf.8. Should be tracked under T333617 and T276237 once deployed.

This patch has been deployed

Thanks. I tested it on wikis where wgParserEnableLegacyMediaDOM is both enabled and disabled,
https://en.wikipedia.org/w/index.php?title=User%3AArlolra%2Fsandbox&diff=1154985575&oldid=1154985505
https://it.wikipedia.org/w/index.php?title=Utente%3AArlolra%2Fsandbox&diff=133524558&oldid=133524549

What's the next step towards getting this merged to master. I'm not sure what that procedure is.

Great, thanks!

What's the next step towards getting this merged to master. I'm not sure what that procedure is.

This patch needs to be held for the next MediaWiki security release (T333616), which should happen sometimes towards the end of June 2023. If we need to rebase a few times in production against new, conflicting patches in gerrit, we can do that.

Noting the lack of rMW6f3b22c41007: Handle thumb errors when !$enableLegacyMediaDOM causes conflicts on REL1_40 (and presumably below)

diff --cc includes/linker/Linker.php
index 562822f1a27,56c22574751..00000000000
--- a/includes/linker/Linker.php
+++ b/includes/linker/Linker.php
@@@ -467,9 -472,12 +467,16 @@@ class Linker 
                        $thumb = false;
                }
  
++<<<<<<< HEAD
 +              if ( !$thumb ) {
++=======
+               $isBadFile = $file && $thumb && $parser &&
+                       $parser->getBadFileLookup()->isBadFile( $title->getDBkey(), $parser->getTitle() );
+ 
+               if ( !$thumb || ( !$enableLegacyMediaDOM && $thumb->isError() ) || $isBadFile ) {
++>>>>>>> b66ca24eefa (SECURITY: Move badFile lookup to Linker)
                        $rdfaType = 'mw:Error ' . $rdfaType;
 -                      $currentExists = $file && $file->exists();
 +                      $label = '';
                        if ( $enableLegacyMediaDOM ) {
                                $label = $frameParams['title'];
                        } else {
@@@ -735,16 -768,24 +742,27 @@@
                                . "<div class=\"thumbinner\" style=\"width:{$outerWidth}px;\">";
                }
  
+               $isBadFile = $exists && $thumb && $parser &&
+                       $parser->getBadFileLookup()->isBadFile(
+                               $manualthumb ? $manual_title : $title->getDBkey(),
+                               $parser->getTitle()
+                       );
+ 
                if ( !$exists ) {
 -                      $rdfaType = 'mw:Error ' . $rdfaType;
                        $label = '';
                        if ( !$enableLegacyMediaDOM ) {
                                $label = $frameParams['alt'] ?? '';
                        }
                        $s .= self::makeBrokenImageLinkObj(
 -                              $title, $label, '', '', '', (bool)$time, $handlerParams, false
 +                              $title, $label, '', '', '', (bool)$time, $handlerParams
                        );
                        $zoomIcon = '';
++<<<<<<< HEAD
 +              } elseif ( !$thumb ) {
++=======
+               } elseif ( !$thumb || ( !$enableLegacyMediaDOM && $thumb->isError() ) || $isBadFile ) {
+                       $rdfaType = 'mw:Error ' . $rdfaType;
++>>>>>>> b66ca24eefa (SECURITY: Move badFile lookup to Linker)
                        if ( $enableLegacyMediaDOM ) {
                                $s .= wfMessage( 'thumbnail_error', '' )->escaped();
                        } else {

Not a big deal... Basically just need to drop the ( !$enableLegacyMediaDOM && $thumb->isError() ) || part.

Amended patch for REL1_40 applies to REL1_39...

It applies to REL1_38 too, but fails on parser tests... Not sure offhand if we can skip changes to that file, or whether CI will fail that.

In REL1_35 tests/parser/mediaParserTests.txt doesn't exist, and there's bigger conflicts in Parser.php and Linker.php...

Will continue poking.

REL1_39/REL1_40 patch:

I'm not quite sure of the parser tests... Most of the changes disappeared from lack of context (which is fine)...

But the mw:Error mw:File/Thumb part.... I guess I probably need to run the tests locally and see if they pass/fail :P

For REL1_35, the separate file isn't there, but https://github.com/wikimedia/mediawiki/blob/REL1_35/tests/parser/parserTests.txt#L15020 would be rough context to add the test (like is left in REL1_38)...

The other diffs...

diff --cc includes/Linker.php
index 39f15ec3104,411d217b0ac..00000000000
--- a/includes/Linker.php
+++ b/includes/Linker.php
@@@ -413,8 -443,20 +413,25 @@@ class Linker 
                        $thumb = false;
                }
  
++<<<<<<< HEAD
 +              if ( !$thumb ) {
 +                      $s = self::makeBrokenImageLinkObj( $title, $frameParams['title'], '', '', '', $time == true );
++=======
+               $isBadFile = $file && $thumb && $parser &&
+                       $parser->getBadFileLookup()->isBadFile( $title->getDBkey(), $parser->getTitle() );
+ 
+               if ( !$thumb || $isBadFile ) {
+                       $rdfaType = 'mw:Error ' . $rdfaType;
+                       $label = '';
+                       if ( $enableLegacyMediaDOM ) {
+                               // This is the information for tooltips for inline images which
+                               // Parsoid stores in data-mw.  See T273014
+                               $label = $frameParams['title'];
+                       }
+                       $s = self::makeBrokenImageLinkObj(
+                               $title, $label, '', '', '', (bool)$time, $handlerParams
+                       );
++>>>>>>> 8e2cd5f5026 (SECURITY: Move badFile lookup to Linker)
                } else {
                        self::processResponsiveImages( $file, $thumb, $handlerParams );
                        $params = [
@@@ -588,17 -695,51 +605,34 @@@
                        && !isset( $frameParams['link-title'] )
                        && !isset( $frameParams['link-url'] )
                        && !isset( $frameParams['no-link'] ) ) {
 -                      $frameParams['link-title'] = $title;
 -                      $frameParams['link-title-query'] = $linkTitleQuery;
 +                      $frameParams['link-url'] = $url;
                }
  
 -              if ( $frameParams['align'] != '' ) {
 -                      // Possible values: mw-halign-left mw-halign-center mw-halign-right mw-halign-none
 -                      $classes[] = "mw-halign-{$frameParams['align']}";
 -              }
 -
 -              if ( isset( $frameParams['class'] ) ) {
 -                      $classes[] = $frameParams['class'];
 -              }
 -
 -              $s = '';
 -
 -              if ( $enableLegacyMediaDOM ) {
 -                      $s .= "<div class=\"thumb t{$frameParams['align']}\">"
 -                              . "<div class=\"thumbinner\" style=\"width:{$outerWidth}px;\">";
 -              }
 +              $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 ) {
 -                      $label = '';
 -                      if ( $enableLegacyMediaDOM ) {
 -                              // This is the information for tooltips for inline images which
 -                              // Parsoid stores in data-mw.  See T273014
 -                              $label = $frameParams['title'];
 -                      }
 -                      $s .= self::makeBrokenImageLinkObj(
 -                              $title, $label, '', '', '', (bool)$time, $handlerParams
 -                      );
 +                      $s .= self::makeBrokenImageLinkObj( $title, $frameParams['title'], '', '', '', $time == true );
                        $zoomIcon = '';
++<<<<<<< HEAD
 +              } elseif ( !$thumb ) {
 +                      $s .= wfMessage( 'thumbnail_error', '' )->escaped();
++=======
+               } elseif ( !$thumb || $isBadFile ) {
+                       if ( $enableLegacyMediaDOM ) {
+                               $s .= wfMessage( 'thumbnail_error', '' )->escaped();
+                       } else {
+                               $s .= self::makeBrokenImageLinkObj(
+                                       $title, '', '', '', '', (bool)$time, $handlerParams
+                               );
+                       }
++>>>>>>> 8e2cd5f5026 (SECURITY: Move badFile lookup to Linker)
                        $zoomIcon = '';
                } else {
                        if ( !$noscale && !$manualthumb ) {
diff --cc includes/parser/Parser.php
index 1295165d0b9,2b0351399fe..00000000000
--- a/includes/parser/Parser.php
+++ b/includes/parser/Parser.php
@@@ -2615,27 -2660,25 +2615,49 @@@ class Parser 
                                        continue;
                                }
  
++<<<<<<< HEAD
 +                              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;
 +                                      }
 +                              } elseif ( $ns == NS_CATEGORY ) {
++=======
+                               if ( $ns === NS_FILE ) {
+                                       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 ) {
++>>>>>>> 8e2cd5f5026 (SECURITY: Move badFile lookup to Linker)
                                        /**
                                         * Strip the whitespace Category links produce, see T2087
                                         */
* Unmerged path tests/parser/mediaParserTests.txt

Not sure why Parser.php is complaining, but it's clearly just whitespace related...

But this won't work, as the $parser parameter into makeThumbLink2 doesn't exist yet... Not a problem for other branches.

Was added in rMWc5c7d5c9bb6d: Fallback to parser-extlink-target instead of setting link-target.

Ignoring the failures in T340390: media parser test failures in REL1_39... In REL1_39

Running test Bad images - basic functionality [legacy]... FAILED!
/var/www/wiki-1.39/core/tests/parser/mediaParserTests.txt:175
--- /tmp/mwParser-expectedigbsbH	2023-06-25 14:23:15.204382604 +0100
+++ /tmp/mwParser-actualJbxaDG	2023-06-25 14:23:15.204382604 +0100
@@ -1,2 +1,2 @@
-<p><span class="mw-default-size" typeof="mw:Error mw:File"><a href="/wiki/File:Bad.jpg" title="File:Bad.jpg"><span class="mw-file-element mw-broken-media" data-width="320">File:Bad.jpg</span></a></span>
+<p><span class="mw-default-size" typeof="mw:File"><a href="/wiki/File:Bad.jpg" class="mw-file-description"><img alt="Bad.jpg" src="http://example.com/images/0/09/Bad.jpg" decoding="async" width="320" height="240" /></a></span>
 </p>
Running test Bad images - T18039: text after bad image disappears [legacy]... FAILED!
/var/www/wiki-1.39/core/tests/parser/mediaParserTests.txt:188
--- /tmp/mwParser-expectedV7V4XX	2023-06-25 14:23:15.220382816 +0100
+++ /tmp/mwParser-actualbn6Wtd	2023-06-25 14:23:15.220382816 +0100
@@ -1,4 +1,4 @@
 <p>Foo bar
-<span class="mw-default-size" typeof="mw:Error mw:File"><a href="/wiki/File:Bad.jpg" title="File:Bad.jpg"><span class="mw-file-element mw-broken-media" data-width="320">File:Bad.jpg</span></a></span>
+<span class="mw-default-size" typeof="mw:File"><a href="/wiki/File:Bad.jpg" class="mw-file-description"><img alt="Bad.jpg" src="http://example.com/images/0/09/Bad.jpg" decoding="async" width="320" height="240" /></a></span>
 Bar foo
 </p>
Running test Bad images - manualthumb [legacy]... FAILED!
/var/www/wiki-1.39/core/tests/parser/mediaParserTests.txt:207
--- /tmp/mwParser-expectedwR8dCd	2023-06-25 14:23:15.236383029 +0100
+++ /tmp/mwParser-actualyNW7Dy	2023-06-25 14:23:15.236383029 +0100
@@ -1 +1 @@
-<figure typeof="mw:Error mw:File/Thumb"><a href="/wiki/File:Foobar.jpg" title="File:Foobar.jpg"><span class="mw-file-element mw-broken-media" data-width="180">Error creating thumbnail: </span></a><figcaption>Uh oh</figcaption></figure>
+<figure typeof="mw:File/Thumb"><a href="/wiki/File:Foobar.jpg" title="File:Foobar.jpg"><img alt="" src="http://example.com/images/0/09/Bad.jpg" decoding="async" width="320" height="240" /></a><figcaption>Uh oh</figcaption></figure>
Running test Bad images - in gallery [legacy]... FAILED!
/var/www/wiki-1.39/core/tests/parser/mediaParserTests.txt:217
--- /tmp/mwParser-expectedEsFPT9	2023-06-25 14:23:15.252383241 +0100
+++ /tmp/mwParser-actualIJs0k7	2023-06-25 14:23:15.252383241 +0100
@@ -1,6 +1,6 @@
 <ul class="gallery mw-gallery-traditional">
 		<li class="gallerybox" style="width: 155px">
-			<div class="thumb" style="height: 150px;"><span typeof="mw:Error mw:File"><a href="/index.php?title=Special:Upload&amp;wpDestFile=Bad.jpg" class="new" title="File:Bad.jpg"><span class="mw-broken-media" data-width="120" data-height="120">File:Bad.jpg</span></a></span></div>
+			<div class="thumb" style="width: 150px; height: 150px;"><span typeof="mw:File"><a href="/wiki/File:Bad.jpg" class="mw-file-description"><img alt="Bad.jpg" src="http://example.com/images/thumb/0/09/Bad.jpg/120px-Bad.jpg" decoding="async" width="120" height="90" srcset="http://example.com/images/thumb/0/09/Bad.jpg/180px-Bad.jpg 1.5x, http://example.com/images/thumb/0/09/Bad.jpg/240px-Bad.jpg 2x" /></a></span></div>
 			<div class="gallerytext">
 			</div>
 		</li>

Which I suspect is due to the other related changes, such as rMWe0035bbf5a14: Fix frame and frameless rdfa depending on file existing which is adding extra $rdfaType = 'mw:Error ' . $rdfaType; into the mix

And https://gerrit.wikimedia.org/r/q/topic:T334659-2 won't apply onto REL1_40... First patch is trivial conflicts, second gets more complex, etc. Similar for REL1_39 too?

I'm a little stuck what to do here....

We can skip parser test changes, which may or may not leave (other) failures behind.

For REL1_35, we could "just" use \MediaWiki\MediaWikiServices::getInstance()->getParser() to gain an instance to play with..

Reedy renamed this task from Manualthumb bypasses badFile lookup to CVE-2023-36674: Manualthumb bypasses badFile lookup.Jun 26 2023, 10:57 AM

Here are the backports for REL1_39. Let me know if that works for you and I'll continue with the rest.

Thanks!

If they're not security patches, they can be done in public to make everything easier :)

I'll stick those first 4 in gerrit in a bit and get them merged too.

I would suggest it's probably not worth doing that full stack (if possible) for REL1_38 as that is basically EOL as of this week. We could just say it's not fixed there.

REL1_35 is EOL in about 3 months too.

Here are the backports for REL1_39. Let me know if that works for you and I'll continue with the rest.

These are now merged into REL1_39 with minor changes (removing a use, adding $ back to a variable), ontop of some mediawiki-phan-config 0.12.0/0.12.1 backports (for ease, plus LTS).

Should mean the security patch is a trivial pick ontop.

I started trying to put the same stack of patches onto REL1_40, but parsertest failure on the first one, so stopped - https://gerrit.wikimedia.org/r/c/mediawiki/core/+/934012

I started trying to put the same stack of patches onto REL1_40, but parsertest failure on the first one, so stopped - https://gerrit.wikimedia.org/r/c/mediawiki/core/+/934012

Here you go,
https://gerrit.wikimedia.org/r/q/topic:T334659-2-REL1_40

I would suggest it's probably not worth doing that full stack (if possible) for REL1_38 as that is basically EOL as of this week. We could just say it's not fixed there.

That's fine

Great, thanks!

I guess this is a low enough severity security fix, for quite a narrow case... This seems fine for me for REL1_35 and REL1_38 to suggest that people upgrade.

I'll have to get the CVE updated later

REL1_35 is EOL in about 3 months too.

Probably reasonable to just backport a variant of the security patch. I'll see what I can do

For REL1_35, we could "just" use \MediaWiki\MediaWikiServices::getInstance()->getParser() to gain an instance to play with..

We just need the badfile service, not the whole parser, I think

That makes me wonder though, the patch I've supplied only considers parser contexts (the check is moved from the parser to the linker) but maybe there are other places where user controlled content make images that should check if it's a bad file?

Oh, I should note, the security patches I uploaded in T335612#8975039 and T335612#8977622 are amended ever so slightly from the original one, to add a test case to the legacy media parsertests file and alter the error output. Here's the same thing that applies to master and is the patch I'd prefer to land there.

Here's the diff between the two,

commit b4d38b335a6d994d3d1645c8a695620299c08a8d
Author: Arlo Breault <abreault@wikimedia.org>
Date:   Fri Apr 28 19:00:50 2023 -0400

    Diff
    
    Change-Id: I42a61870e8ff8d49d6566531a410557382a94800

diff --git a/includes/linker/Linker.php b/includes/linker/Linker.php
index 56c22574751..c53a412d9c1 100644
--- a/includes/linker/Linker.php
+++ b/includes/linker/Linker.php
@@ -787,7 +787,13 @@ class Linker {
 		} elseif ( !$thumb || ( !$enableLegacyMediaDOM && $thumb->isError() ) || $isBadFile ) {
 			$rdfaType = 'mw:Error ' . $rdfaType;
 			if ( $enableLegacyMediaDOM ) {
-				$s .= wfMessage( 'thumbnail_error', '' )->escaped();
+				if ( !$thumb ) {
+					$s .= wfMessage( 'thumbnail_error', '' )->escaped();
+				} else {
+					$s .= self::makeBrokenImageLinkObj(
+						$title, '', '', '', '', (bool)$time, $handlerParams, true
+					);
+				}
 			} else {
 				if ( $thumb && $thumb->isError() ) {
 					Assert::invariant(
@@ -795,8 +801,10 @@ class Linker {
 						'Unknown MediaTransformOutput: ' . get_class( $thumb )
 					);
 					$label = $thumb->toText();
-				} else {
+				} elseif ( !$thumb ) {
 					$label = wfMessage( 'thumbnail_error', '' )->text();
+				} else {
+					$label = '';
 				}
 				$s .= self::makeBrokenImageLinkObj(
 					$title, $label, '', '', '', (bool)$time, $handlerParams, true
diff --git a/tests/parser/legacyMedia.txt b/tests/parser/legacyMedia.txt
index 3cad1ab784d..863ff3c77c1 100644
--- a/tests/parser/legacyMedia.txt
+++ b/tests/parser/legacyMedia.txt
@@ -218,6 +218,16 @@ Bar foo
 Bar foo</p>
 !! end
 
+!! test
+Bad images - manualthumb
+!! config
+wgParserEnableLegacyMediaDOM=true
+!! wikitext
+[[File:Foobar.jpg|thumb=Bad.jpg|Uh oh]]
+!! html/php
+<div class="thumb tright"><div class="thumbinner" style="width:322px;"><a href="/wiki/File:Foobar.jpg" title="File:Foobar.jpg">File:Foobar.jpg</a>  <div class="thumbcaption">Uh oh</div></div></div>
+!! end
+
 !! test
 Bad images - in gallery
 !! config
diff --git a/tests/parser/media.txt b/tests/parser/media.txt
index 902deac44a8..beb01e33b62 100644
--- a/tests/parser/media.txt
+++ b/tests/parser/media.txt
@@ -235,7 +235,7 @@ wgParserEnableLegacyMediaDOM=false
 !! wikitext
 [[File:Foobar.jpg|thumb=Bad.jpg|Uh oh]]
 !! html/php
-<figure typeof="mw:Error mw:File/Thumb"><a href="/wiki/File:Foobar.jpg" title="File:Foobar.jpg"><span class="mw-file-element mw-broken-media" data-width="180">Error creating thumbnail: </span></a><figcaption>Uh oh</figcaption></figure>
+<figure typeof="mw:Error mw:File/Thumb"><a href="/wiki/File:Foobar.jpg" title="File:Foobar.jpg"><span class="mw-file-element mw-broken-media" data-width="180">File:Foobar.jpg</span></a><figcaption>Uh oh</figcaption></figure>
 !! end
 
 !! test

For REL1_35, we could "just" use \MediaWiki\MediaWikiServices::getInstance()->getParser() to gain an instance to play with..

We just need the badfile service, not the whole parser, I think

Ah, but then there's no article title context. Here's a backport to REL1_35, if you want it. Unfortunately, the tests were still disabled at the time of this release but I tried it out locally.

For REL1_35, we could "just" use \MediaWiki\MediaWikiServices::getInstance()->getParser() to gain an instance to play with..

We just need the badfile service, not the whole parser, I think

Ah, but then there's no article title context. Here's a backport to REL1_35, if you want it. Unfortunately, the tests were still disabled at the time of this release but I tried it out locally.

Thanks

I suspect phan or something will complain about the && $parser on https://phabricator.wikimedia.org/F37123435$31 because it's not nullable, so it should always be true-y.

Easily fixed, so will try and run phan locally and see... And then amend if necessary.

I've just forward ported that to REL1_38, unclear if it's going to break any parser tests...

(untested)

TBH, it might be worth getting CI to run and merge the patches in the open before I actually build the releases

I suspect phan or something will complain about the && $parser on https://phabricator.wikimedia.org/F37123435$31 because it's not nullable, so it should always be true-y.

Easily fixed, so will try and run phan locally and see... And then amend if necessary.

includes/Linker.php:416 PhanRedundantCondition Redundant attempt to cast $parser of type \Parser to truthy

Heh, yup.

Passes if I remove that :)

I've just forward ported that to REL1_38, unclear if it's going to break any parser tests...

The bad image tests are disabled in REL1_38, so probably not

I suspect phan or something will complain about the && $parser on https://phabricator.wikimedia.org/F37123435$31 because it's not nullable, so it should always be true-y.

Ugh 😔 That's in all the patches

If REL1_38 is probably ok, might aswell include the patch then. Gives us a clean sweep.

The && $parser is easily fixed anyway :) - I'll sort it when I'm staging them tomorrow

Ok, thank you, and sorry for the mess

Don't worry. You've more than helped sorting the backports :)

Change 934563 merged by jenkins-bot:

[mediawiki/core@REL1_35] SECURITY: Move badFile lookup to Linker

https://gerrit.wikimedia.org/r/934563

Change 934566 merged by jenkins-bot:

[mediawiki/core@REL1_38] SECURITY: Move badFile lookup to Linker

https://gerrit.wikimedia.org/r/934566

Change 934568 merged by jenkins-bot:

[mediawiki/core@REL1_39] SECURITY: Move badFile lookup to Linker

https://gerrit.wikimedia.org/r/934568

Change 934571 merged by jenkins-bot:

[mediawiki/core@master] SECURITY: Move badFile lookup to Linker

https://gerrit.wikimedia.org/r/934571

Reedy assigned this task to Arlolra.

Change 934584 had a related patch set uploaded (by Reedy; author: Arlolra):

[mediawiki/core@REL1_40] SECURITY: Move badFile lookup to Linker

https://gerrit.wikimedia.org/r/934584

Change 934584 merged by jenkins-bot:

[mediawiki/core@REL1_40] SECURITY: Move badFile lookup to Linker

https://gerrit.wikimedia.org/r/934584

Reedy changed the visibility from "Custom Policy" to "Public (No Login Required)".
Reedy changed the edit policy from "Custom Policy" to "All Users".