Page MenuHomePhabricator

img_auth.php may leak private extension images into the public cache (CVE-2020-15005)
Closed, ResolvedPublicSecurity

Description

While working on T235357, I noticed that img_auth.php calls FileBackend::streamFile() with incorrect parameters. Instead of passing the headers as a member of $params, it tries to pass it as a non-existent second parameter. The bug has probably existed since the feature was introduced in 0eb52399689df5fd1401eab951f3a8ef460f7665. The author probably confused the signature with FileRepo::streamFile(), which does take headers as the second parameter.

This occurs only when streaming images from directories that are configured with $wgImgAuthUrlPathMap. I note that in private wikis in WMF production, only EasyTimeline is configured in this way. So in simple terms, for WMF, the risk is that after an authorized user views an EasyTimeline image, it may be cached in ATS, so that an unauthorized user may download the file by requesting the same URL.

The fix was in PS1 of https://gerrit.wikimedia.org/r/c/mediawiki/core/+/584817 , but I backed it out when I realised that it might be treated as a security issue.

I tested the issue locally and confirmed the lack of CC headers, using a simple FSFileBackend to handle a path, not EasyTimeline. I confirmed that the headers are sent with the fix quoted below.

commit 397092d24348e942b54205c0b77ec7b80bbd385a (HEAD -> bug/T235357)
Author: Tim Starling <tstarling@wikimedia.org>
Date:   Tue Mar 31 17:02:49 2020 +1100

    Fix accidental public CC headers in img_auth.php
    
    Incorrect parameters to FileBackend::streamFile() caused
    Cache-Control:private and Vary:Cookie response headers to be omitted
    when requesting a file in a path configured by $wgImgAuthUrlPathMap.
    Typically this is used to deliver images generated by extensions.
    
    Change-Id: I404d9462e4b35d3d832bfab21954ff87e46e3eb2

diff --git a/img_auth.php b/img_auth.php
index b3f82018eb..74f9cc2e13 100644
--- a/img_auth.php
+++ b/img_auth.php
@@ -99,8 +99,10 @@ function wfImageAuthMain() {
 			}
 			if ( $be->fileExists( [ 'src' => $filename ] ) ) {
 				wfDebugLog( 'img_auth', "Streaming `" . $filename . "`." );
-				$be->streamFile( [ 'src' => $filename ],
-					[ 'Cache-Control: private', 'Vary: Cookie' ] );
+				$be->streamFile( [
+					'src' => $filename,
+					'headers' => [ 'Cache-Control: private', 'Vary: Cookie' ]
+				] );
 			} else {
 				wfForbidden( 'img-auth-accessdenied', 'img-auth-nofile', $path );
 			}

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 31 2020, 6:24 AM
Restricted Application added a project: Commons. · View Herald TranscriptMar 31 2020, 7:08 AM
Reedy triaged this task as Medium priority.Apr 6 2020, 3:20 PM
Reedy added a subscriber: Reedy.

I'll get this deployed tonight.

I note that in private wikis in WMF production, only EasyTimeline is configured in this way. So in simple terms, for WMF, the risk is that after an authorized user views an EasyTimeline image, it may be cached in ATS, so that an unauthorized user may download the file by requesting the same URL

I note for WMF as per the above, the cases of this happening is probably low.

Possibly best not putting this into public until the next security release.. Or maybe we could as it's a narrow case?

Reedy moved this task from Incoming to In Progress on the Security-Team board.Apr 6 2020, 3:21 PM
Reedy added a comment.Apr 6 2020, 9:48 PM

Deployed.

From commit 397092d24348e942b54205c0b77ec7b80bbd385a (HEAD -> bug/T235357)
From: Tim Starling <tstarling@wikimedia.org>
Date: Tue Mar 31 17:02:49 2020 +1100
Subject: SECURITY: Fix accidental public CC headers in img_auth.php
    
Incorrect parameters to FileBackend::streamFile() caused
Cache-Control:private and Vary:Cookie response headers to be omitted
when requesting a file in a path configured by $wgImgAuthUrlPathMap.
Typically this is used to deliver images generated by extensions.
    
Change-Id: I404d9462e4b35d3d832bfab21954ff87e46e3eb2

diff --git a/img_auth.php b/img_auth.php
index b3f82018eb..74f9cc2e13 100644
--- a/img_auth.php
+++ b/img_auth.php
@@ -99,8 +99,10 @@ function wfImageAuthMain() {
 			}
 			if ( $be->fileExists( [ 'src' => $filename ] ) ) {
 				wfDebugLog( 'img_auth', "Streaming `" . $filename . "`." );
-				$be->streamFile( [ 'src' => $filename ],
-					[ 'Cache-Control: private', 'Vary: Cookie' ] );
+				$be->streamFile( [
+					'src' => $filename,
+					'headers' => [ 'Cache-Control: private', 'Vary: Cookie' ]
+				] );
 			} else {
 				wfForbidden( 'img-auth-accessdenied', 'img-auth-nofile', $path );
 			}
sbassett moved this task from In Progress to Watching on the Security-Team board.Apr 7 2020, 4:19 PM
Krinkle added a subscriber: Krinkle.Apr 8 2020, 8:36 PM
Krinkle removed a subscriber: Krinkle.Sat, Jun 20, 11:02 PM
Legoktm renamed this task from img_auth.php may leak private extension images into the public cache to img_auth.php may leak private extension images into the public cache (CVE-2020-15005).Wed, Jun 24, 11:20 AM
Reedy changed the visibility from "Custom Policy" to "Public (No Login Required)".Wed, Jun 24, 4:56 PM
Reedy changed the edit policy from "Custom Policy" to "All Users".

Change 607557 had a related patch set uploaded (by Reedy; owner: Tim Starling):
[mediawiki/core@REL1_31] SECURITY: Fix accidental public CC headers in img_auth.php

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

Change 607560 had a related patch set uploaded (by Reedy; owner: Tim Starling):
[mediawiki/core@REL1_33] SECURITY: Fix accidental public CC headers in img_auth.php

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

Change 607562 had a related patch set uploaded (by Reedy; owner: Tim Starling):
[mediawiki/core@REL1_34] SECURITY: Fix accidental public CC headers in img_auth.php

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

Change 607565 had a related patch set uploaded (by Reedy; owner: Tim Starling):
[mediawiki/core@master] SECURITY: Fix accidental public CC headers in img_auth.php

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

Reedy closed this task as Resolved.Wed, Jun 24, 5:00 PM
Reedy assigned this task to tstarling.

Change 607557 merged by jenkins-bot:
[mediawiki/core@REL1_31] SECURITY: Fix accidental public CC headers in img_auth.php

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

Change 607562 merged by jenkins-bot:
[mediawiki/core@REL1_34] SECURITY: Fix accidental public CC headers in img_auth.php

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

Change 607560 merged by Reedy:
[mediawiki/core@REL1_33] SECURITY: Fix accidental public CC headers in img_auth.php

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

Change 607565 merged by jenkins-bot:
[mediawiki/core@master] SECURITY: Fix accidental public CC headers in img_auth.php

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

hashar added a subscriber: hashar.Tue, Jun 30, 8:50 AM

I have removed the patch for 1.35.0-wmf.39 (T254176) since it has been released:

rm /srv/patches/1.35.0-wmf.39/core/02-T248947.patch