Page MenuHomePhabricator

omnibus-v3-REL1_24.patch

Authored By
demon
Dec 17 2015, 9:08 PM
Size
35 KB
Referenced Files
None
Subscribers
None

omnibus-v3-REL1_24.patch

From d18ea364a0bdd934c56a4e335a96f44bbf866d7c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gerg=C5=91=20Tisza?= <tgr.huwiki@gmail.com>
Date: Sat, 21 Nov 2015 11:51:02 -0800
Subject: [PATCH 01/10] Use hash_equals in User::matchEditToken
There is no point in using hash_equals for the return value if we
do a normal comparison before.
Bug: T119309
Change-Id: Ia44ec5ed492105b27d0fddd845d58d27a29dc072
Signed-off-by: Chad Horohoe <chadh@wikimedia.org>
---
includes/User.php | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/includes/User.php b/includes/User.php
index a925a3c..0a5c5da 100644
--- a/includes/User.php
+++ b/includes/User.php
@@ -3932,11 +3932,12 @@ class User implements IDBAccessObject {
*/
public function matchEditToken( $val, $salt = '', $request = null ) {
$sessionToken = $this->getEditToken( $salt, $request );
- if ( $val != $sessionToken ) {
+ $equals = hash_equals( $sessionToken, $val );
+ if ( !$equals ) {
wfDebug( "User::matchEditToken: broken session data\n" );
}
- return $val == $sessionToken;
+ return $equals;
}
/**
@@ -3950,7 +3951,7 @@ class User implements IDBAccessObject {
*/
public function matchEditTokenNoSuffix( $val, $salt = '', $request = null ) {
$sessionToken = $this->getEditToken( $salt, $request );
- return substr( $sessionToken, 0, 32 ) == substr( $val, 0, 32 );
+ return hash_equals( substr( $sessionToken, 0, 32 ), substr( $val, 0, 32 ) );
}
/**
--
2.6.4
From 14b825925c4208afe04dd61736e9fc60aec64f54 Mon Sep 17 00:00:00 2001
From: Roan Kattouw <roan.kattouw@gmail.com>
Date: Fri, 6 Nov 2015 12:55:16 -0800
Subject: [PATCH 02/10] SECURITY: Work around CURL insanity breaking POST
parameters that start with '@'
CURL has a "feature" where passing array( 'foo' => '@bar' )
in CURLOPT_POSTFIELDS results in the contents of the file named "bar"
being POSTed. This makes it impossible to POST the literal string "@bar",
because array( 'foo' => '%40bar' ) gets double-encoded to foo=%2540bar.
Disable this "feature" by setting CURLOPT_SAFE_UPLOAD to true,
if available. According to the PHP manual, this option became
available in 5.5 and started defaulting to true in 5.6.
However, we support versions as low as 5.3, and this option
doesn't exist at all in 5.6.99-hhvm, which we run in production.
For versions where this option is not available (pre-5.5 versions
and HHVM), serialize POSTFIELDS arrays to strings. This works
around the issue because the '@' "feature" only works
for arrays, not strings, as of PHP 5.2. (We don't support pre-5.2
versions, and I've verified 5.6.99-hhvm behaves this way as well.)
Bug: T118032
Change-Id: I3f996e2eb87c7bd3b94ca9d3cc14a3e12f34f241
Signed-off-by: Chad Horohoe <chadh@wikimedia.org>
---
includes/HttpFunctions.php | 17 ++++++++++++++++-
includes/libs/MultiHttpClient.php | 13 +++++++++++++
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/includes/HttpFunctions.php b/includes/HttpFunctions.php
index bed79dd..17d222d 100644
--- a/includes/HttpFunctions.php
+++ b/includes/HttpFunctions.php
@@ -758,7 +758,22 @@ class CurlHttpRequest extends MWHttpRequest {
$this->curlOptions[CURLOPT_HEADER] = true;
} elseif ( $this->method == 'POST' ) {
$this->curlOptions[CURLOPT_POST] = true;
- $this->curlOptions[CURLOPT_POSTFIELDS] = $this->postData;
+ $postData = $this->postData;
+ // Don't interpret POST parameters starting with '@' as file uploads, because this
+ // makes it impossible to POST plain values starting with '@' (and causes security
+ // issues potentially exposing the contents of local files).
+ // The PHP manual says this option was introduced in PHP 5.5 defaults to true in PHP 5.6,
+ // but we support lower versions, and the option doesn't exist in HHVM 5.6.99.
+ if ( defined( 'CURLOPT_SAFE_UPLOAD' ) ) {
+ $this->curlOptions[CURLOPT_SAFE_UPLOAD] = true;
+ } else if ( is_array( $postData ) ) {
+ // In PHP 5.2 and later, '@' is interpreted as a file upload if POSTFIELDS
+ // is an array, but not if it's a string. So convert $req['body'] to a string
+ // for safety.
+ $postData = wfArrayToCgi( $postData );
+ }
+ $this->curlOptions[CURLOPT_POSTFIELDS] = $postData;
+
// Suppress 'Expect: 100-continue' header, as some servers
// will reject it with a 417 and Curl won't auto retry
// with HTTP 1.0 fallback
diff --git a/includes/libs/MultiHttpClient.php b/includes/libs/MultiHttpClient.php
index 8c982c4..f2088d5 100644
--- a/includes/libs/MultiHttpClient.php
+++ b/includes/libs/MultiHttpClient.php
@@ -310,6 +310,19 @@ class MultiHttpClient {
);
} elseif ( $req['method'] === 'POST' ) {
curl_setopt( $ch, CURLOPT_POST, 1 );
+ // Don't interpret POST parameters starting with '@' as file uploads, because this
+ // makes it impossible to POST plain values starting with '@' (and causes security
+ // issues potentially exposing the contents of local files).
+ // The PHP manual says this option was introduced in PHP 5.5 defaults to true in PHP 5.6,
+ // but we support lower versions, and the option doesn't exist in HHVM 5.6.99.
+ if ( defined( 'CURLOPT_SAFE_UPLOAD' ) ) {
+ curl_setopt( $ch, CURLOPT_SAFE_UPLOAD, true );
+ } else if ( is_array( $req['body'] ) ) {
+ // In PHP 5.2 and later, '@' is interpreted as a file upload if POSTFIELDS
+ // is an array, but not if it's a string. So convert $req['body'] to a string
+ // for safety.
+ $req['body'] = wfArrayToCgi( $req['body'] );
+ }
curl_setopt( $ch, CURLOPT_POSTFIELDS, $req['body'] );
} else {
if ( is_resource( $req['body'] ) || $req['body'] !== '' ) {
--
2.6.4
From e840a771a21f4cde13f2124ccf4aeb9d4aaff6a5 Mon Sep 17 00:00:00 2001
From: Brad Jorsch <bjorsch@wikimedia.org>
Date: Wed, 14 Oct 2015 17:53:09 -0400
Subject: [PATCH 03/10] [SECURITY] 0-pad to length in random string generation
Otherwise shorter strings might be generated.
Bug: T115522
Change-Id: I3569218ea840e9de7a3fe458acf474e3dac6d1ab
Signed-off-by: Chad Horohoe <chadh@wikimedia.org>
---
includes/User.php | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/includes/User.php b/includes/User.php
index 0a5c5da..f287263 100644
--- a/includes/User.php
+++ b/includes/User.php
@@ -991,11 +991,10 @@ class User implements IDBAccessObject {
// stopping at a minimum of 10 chars.
$length = max( 10, $wgMinimalPasswordLength );
// Multiply by 1.25 to get the number of hex characters we need
- $length = $length * 1.25;
// Generate random hex chars
- $hex = MWCryptRand::generateHex( $length );
+ $hex = MWCryptRand::generateHex( ceil( $length * 1.25 ) );
// Convert from base 16 to base 32 to get a proper password like string
- return wfBaseConvert( $hex, 16, 32 );
+ return substr( wfBaseConvert( $hex, 16, 32, $length ), -$length );
}
/**
--
2.6.4
From 36ca966bbdb1a83f9a5f826572a132227ea5d0d2 Mon Sep 17 00:00:00 2001
From: Marius Hoch <hoo@online.de>
Date: Sat, 2 May 2015 18:48:04 +0200
Subject: [PATCH 04/10] Fix IP::toHex for IPv4 addresses with a double/triple 0
block
Bug: T97897
Change-Id: I5c0a37be42ae2c5091ead487a6d19f6e0dd89b36
Signed-off-by: Chad Horohoe <chadh@wikimedia.org>
---
includes/utils/IP.php | 22 ++++++++++++++++------
tests/phpunit/includes/utils/IPTest.php | 33 ++++++++++++++++++++++++++++-----
2 files changed, 44 insertions(+), 11 deletions(-)
diff --git a/includes/utils/IP.php b/includes/utils/IP.php
index 0e2db8c..d1f918b 100644
--- a/includes/utils/IP.php
+++ b/includes/utils/IP.php
@@ -130,8 +130,9 @@ class IP {
/**
* Convert an IP into a verbose, uppercase, normalized form.
- * IPv6 addresses in octet notation are expanded to 8 words.
- * IPv4 addresses are just trimmed.
+ * Both IPv4 and IPv6 addresses are trimmed. Additionally,
+ * IPv6 addresses in octet notation are expanded to 8 words;
+ * IPv4 addresses have leading zeros, in each octet, removed.
*
* @param string $ip IP address in quad or octet form (CIDR or not).
* @return string
@@ -141,8 +142,16 @@ class IP {
if ( $ip === '' ) {
return null;
}
- if ( self::isIPv4( $ip ) || !self::isIPv6( $ip ) ) {
- return $ip; // nothing else to do for IPv4 addresses or invalid ones
+ /* If not an IP, just return trimmed value, since sanitizeIP() is called
+ * in a number of contexts where usernames are supplied as input.
+ */
+ if ( !self::isIPAddress($ip) ) {
+ return $ip;
+ }
+ if ( self::isIPv4( $ip ) ) {
+ // Remove leading 0's from octet representation of IPv4 address
+ $ip = preg_replace( '/(?:^|(?<=\.))0+(?=[1-9]|0\.|0$)/', '', $ip );
+ return $ip;
}
// Remove any whitespaces, convert to upper case
$ip = strtoupper( $ip );
@@ -395,8 +404,9 @@ class IP {
if ( self::isIPv6( $ip ) ) {
$n = 'v6-' . self::IPv6ToRawHex( $ip );
} elseif ( self::isIPv4( $ip ) ) {
- // Bug 60035: an IP with leading 0's fails in ip2long sometimes (e.g. *.08)
- $ip = preg_replace( '/(?<=\.)0+(?=[1-9])/', '', $ip );
+ // T62035/T97897: An IP with leading 0's fails in ip2long sometimes (e.g. *.08),
+ // also double/triple 0 needs to be changed to just a single 0 for ip2long.
+ $ip = self::sanitizeIP( $ip );
$n = ip2long( $ip );
if ( $n < 0 ) {
$n += pow( 2, 32 );
diff --git a/tests/phpunit/includes/utils/IPTest.php b/tests/phpunit/includes/utils/IPTest.php
index ebe347f..5404bb3 100644
--- a/tests/phpunit/includes/utils/IPTest.php
+++ b/tests/phpunit/includes/utils/IPTest.php
@@ -270,12 +270,34 @@ class IPTest extends MediaWikiTestCase {
}
/**
- * Improve IP::sanitizeIP() code coverage
- * @todo Most probably incomplete
+ * @covers IP::sanitizeIP
+ * @dataProvider provideSanitizeIP
*/
- public function testSanitizeIP() {
- $this->assertNull( IP::sanitizeIP( '' ) );
- $this->assertNull( IP::sanitizeIP( ' ' ) );
+ public function testSanitizeIP( $expected, $input ) {
+ $result = IP::sanitizeIP( $input );
+ $this->assertEquals( $expected, $result );
+ }
+
+ /**
+ * Provider for IP::testSanitizeIP()
+ */
+ public static function provideSanitizeIP() {
+ return array(
+ array( '0.0.0.0', '0.0.0.0' ),
+ array( '0.0.0.0', '00.00.00.00' ),
+ array( '0.0.0.0', '000.000.000.000' ),
+ array( '141.0.11.253', '141.000.011.253' ),
+ array( '1.2.4.5', '1.2.4.5' ),
+ array( '1.2.4.5', '01.02.04.05' ),
+ array( '1.2.4.5', '001.002.004.005' ),
+ array( '10.0.0.1', '010.0.000.1' ),
+ array( '80.72.250.4', '080.072.250.04' ),
+ array( 'Foo.1000.00', 'Foo.1000.00'),
+ array( 'Bar.01', 'Bar.01'),
+ array( 'Bar.010', 'Bar.010'),
+ array( null, ''),
+ array( null, ' ')
+ );
}
/**
@@ -299,6 +321,7 @@ class IPTest extends MediaWikiTestCase {
array( '80000000', '128.0.0.0' ),
array( 'DEADCAFE', '222.173.202.254' ),
array( 'FFFFFFFF', '255.255.255.255' ),
+ array( '8D000BFD', '141.000.11.253' ),
array( false, 'IN.VA.LI.D' ),
array( 'v6-00000000000000000000000000000001', '::1' ),
array( 'v6-20010DB885A3000000008A2E03707334', '2001:0db8:85a3:0000:0000:8a2e:0370:7334' ),
--
2.6.4
From 63d7ce7c012bd01bb88092fcb57a41607bc35ac5 Mon Sep 17 00:00:00 2001
From: JuneHyeon Bae <devunt@gmail.com>
Date: Sat, 24 May 2014 19:48:02 +0900
Subject: [PATCH 05/10] Validates wgArticlePath does start with slash (/).
When relative URL used in $wgArticlePath, and $wgArticlePath does not
start with slash (/), raise FatalError.
Bug: T48998
Change-Id: Ic7cd6f774cff97081f4f35af351161170b4b26eb
---
includes/Setup.php | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/includes/Setup.php b/includes/Setup.php
index 3190da6..192f3b9 100644
--- a/includes/Setup.php
+++ b/includes/Setup.php
@@ -479,6 +479,20 @@ require_once "$IP/includes/normal/UtfNormalUtil.php";
require_once "$IP/includes/normal/UtfNormalDefines.php";
wfProfileOut( $fname . '-includes' );
+wfProfileIn( $fname . '-validation' );
+
+// T48998: Bail out early if $wgArticlePath is non-absolute
+if ( !preg_match( '/^https?:\/\/|\//', $wgArticlePath ) ) {
+ throw new FatalError(
+ 'If you use a relative URL on $wgArticlePath, it must start ' .
+ 'with a slash (/).<br><br>See ' .
+ '<a href="https://www.mediawiki.org/wiki/Manual:$wgArticlePath">' .
+ 'https://www.mediawiki.org/wiki/Manual:$wgArticlePath</a>'
+ );
+}
+
+wfProfileOut( $fname . '-validation' );
+
wfProfileIn( $fname . '-defaults2' );
if ( $wgCanonicalServer === false ) {
--
2.6.4
From cbded851bcb6247b63a2baddc816cd1946443140 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bartosz=20Dziewo=C5=84ski?= <matma.rex@gmail.com>
Date: Wed, 11 Nov 2015 23:00:31 +0100
Subject: [PATCH 06/10] Really validate that $wgArticlePath starts with a slash
The regular expression wasn't entirely correct.
Follow-up to a4a3d0454069c25a24e2bfe732a665cc6a865878.
Bug: T48998
Change-Id: I08bdf2db20c1c3de55527fc812bcbb55fa23f7bc
---
includes/Setup.php | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/includes/Setup.php b/includes/Setup.php
index 192f3b9..5801796 100644
--- a/includes/Setup.php
+++ b/includes/Setup.php
@@ -482,12 +482,12 @@ wfProfileOut( $fname . '-includes' );
wfProfileIn( $fname . '-validation' );
// T48998: Bail out early if $wgArticlePath is non-absolute
-if ( !preg_match( '/^https?:\/\/|\//', $wgArticlePath ) ) {
+if ( !preg_match( '/^(https?:\/\/|\/)/', $wgArticlePath ) ) {
throw new FatalError(
- 'If you use a relative URL on $wgArticlePath, it must start ' .
- 'with a slash (/).<br><br>See ' .
+ 'If you use a relative URL for $wgArticlePath, it must start ' .
+ 'with a slash (<code>/</code>).<br><br>See ' .
'<a href="https://www.mediawiki.org/wiki/Manual:$wgArticlePath">' .
- 'https://www.mediawiki.org/wiki/Manual:$wgArticlePath</a>'
+ 'https://www.mediawiki.org/wiki/Manual:$wgArticlePath</a>.'
);
}
--
2.6.4
From 773b49dccdfeaa332553390c7f467d8dd90f21eb Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinklemail@gmail.com>
Date: Fri, 19 Jun 2015 20:56:36 +0100
Subject: [PATCH 07/10] MediaWiki.php: Factor out tryNormaliseRedirect
This is in preparation for fixing T67402, which requires adding
logic inside this condition block. However the to-be-added code
will influences whether or not a redirect should be made.
In case a redirect is not made, it has to fall through to the next
'elseif' handler in MediaWiki::performRequest(), which is not possible
from inside the 'if' block.
Hence, move it out in a separate block and use a boolean return value
to communicate whether the case has been handled.
This also allows us to unit test this thing. Which is desperately
needed. Albeit ugly as it requires lots of mocking.
Change-Id: If3157f2ff1fd3ab2ca20a5d1f550d864ea62c493
---
includes/MediaWiki.php | 145 ++++++++++++++++------------
tests/phpunit/includes/MediaWikiTest.php | 157 +++++++++++++++++++++++++++++++
2 files changed, 241 insertions(+), 61 deletions(-)
create mode 100644 tests/phpunit/includes/MediaWikiTest.php
diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php
index 402494e..e06877f 100644
--- a/includes/MediaWiki.php
+++ b/includes/MediaWiki.php
@@ -225,76 +225,99 @@ class MediaWiki {
$output->redirect( $url, 301 );
} else {
$this->context->setTitle( SpecialPage::getTitleFor( 'Badtitle' ) );
- wfProfileOut( __METHOD__ );
throw new BadTitleError();
}
- // Redirect loops, no title in URL, $wgUsePathInfo URLs, and URLs with a variant
- } elseif ( $request->getVal( 'action', 'view' ) == 'view' && !$request->wasPosted()
- && ( $request->getVal( 'title' ) === null
- || $title->getPrefixedDBkey() != $request->getVal( 'title' ) )
- && !count( $request->getValueNames( array( 'action', 'title' ) ) )
- && wfRunHooks( 'TestCanonicalRedirect', array( $request, $title, $output ) )
- ) {
- if ( $title->isSpecialPage() ) {
- list( $name, $subpage ) = SpecialPageFactory::resolveAlias( $title->getDBkey() );
- if ( $name ) {
- $title = SpecialPage::getTitleFor( $name, $subpage );
- }
- }
- $targetUrl = wfExpandUrl( $title->getFullURL(), PROTO_CURRENT );
- // Redirect to canonical url, make it a 301 to allow caching
- if ( $targetUrl == $request->getFullRequestURL() ) {
- $message = "Redirect loop detected!\n\n" .
- "This means the wiki got confused about what page was " .
- "requested; this sometimes happens when moving a wiki " .
- "to a new server or changing the server configuration.\n\n";
-
- if ( $this->config->get( 'UsePathInfo' ) ) {
- $message .= "The wiki is trying to interpret the page " .
- "title from the URL path portion (PATH_INFO), which " .
- "sometimes fails depending on the web server. Try " .
- "setting \"\$wgUsePathInfo = false;\" in your " .
- "LocalSettings.php, or check that \$wgArticlePath " .
- "is correct.";
+ // Handle any other redirects.
+ // Redirect loops, titleless URL, $wgUsePathInfo URLs, and URLs with a variant
+ } elseif ( !$this->tryNormaliseRedirect( $title ) ) {
+
+ // Special pages
+ if ( NS_SPECIAL == $title->getNamespace() ) {
+ // Actions that need to be made when we have a special pages
+ SpecialPageFactory::executePath( $title, $this->context );
+ } else {
+ // ...otherwise treat it as an article view. The article
+ // may still be a wikipage redirect to another article or URL.
+ $article = $this->initializeArticle();
+ if ( is_object( $article ) ) {
+ $this->performAction( $article, $requestTitle );
+ } elseif ( is_string( $article ) ) {
+ $output->redirect( $article );
} else {
- $message .= "Your web server was detected as possibly not " .
- "supporting URL path components (PATH_INFO) correctly; " .
- "check your LocalSettings.php for a customized " .
- "\$wgArticlePath setting and/or toggle \$wgUsePathInfo " .
- "to true.";
+ throw new MWException( "Shouldn't happen: MediaWiki::initializeArticle()"
+ . " returned neither an object nor a URL" );
}
- throw new HttpError( 500, $message );
- } else {
- $output->setSquidMaxage( 1200 );
- $output->redirect( $targetUrl, '301' );
- }
- // Special pages
- } elseif ( NS_SPECIAL == $title->getNamespace() ) {
- $pageView = true;
- // Actions that need to be made when we have a special pages
- SpecialPageFactory::executePath( $title, $this->context );
- } else {
- // ...otherwise treat it as an article view. The article
- // may be a redirect to another article or URL.
- $article = $this->initializeArticle();
- if ( is_object( $article ) ) {
- $pageView = true;
- $this->performAction( $article, $requestTitle );
- } elseif ( is_string( $article ) ) {
- $output->redirect( $article );
- } else {
- wfProfileOut( __METHOD__ );
- throw new MWException( "Shouldn't happen: MediaWiki::initializeArticle()"
- . " returned neither an object nor a URL" );
}
}
+ }
- if ( $pageView ) {
- // Promote user to any groups they meet the criteria for
- $user->addAutopromoteOnceGroups( 'onView' );
+ /**
+ * Handle redirects for uncanonical title requests.
+ *
+ * Handles:
+ * - Redirect loops.
+ * - No title in URL.
+ * - $wgUsePathInfo URLs.
+ * - URLs with a variant.
+ * - Other non-standard URLs (as long as they have no extra query parameters).
+ *
+ * Behaviour:
+ * - Normalise title values:
+ * /wiki/Foo%20Bar -> /wiki/Foo_Bar
+ * - Normalise empty title:
+ * /wiki/ -> /wiki/Main
+ * /w/index.php?title= -> /wiki/Main
+ * - Don't redirect anything with query parameters other than 'title' or 'action=view'.
+ *
+ * @return bool True if a redirect was set.
+ */
+ private function tryNormaliseRedirect( $title ) {
+ $request = $this->context->getRequest();
+ $output = $this->context->getOutput();
+
+ if ( $request->getVal( 'action', 'view' ) != 'view'
+ || $request->wasPosted()
+ || ( $request->getVal( 'title' ) !== null
+ && $title->getPrefixedDBkey() == $request->getVal( 'title' ) )
+ || count( $request->getValueNames( array( 'action', 'title' ) ) )
+ || !Hooks::run( 'TestCanonicalRedirect', array( $request, $title, $output ) )
+ ) {
+ return false;
}
- wfProfileOut( __METHOD__ );
+ if ( $title->isSpecialPage() ) {
+ list( $name, $subpage ) = SpecialPageFactory::resolveAlias( $title->getDBkey() );
+ if ( $name ) {
+ $title = SpecialPage::getTitleFor( $name, $subpage );
+ }
+ }
+ // Redirect to canonical url, make it a 301 to allow caching
+ $targetUrl = wfExpandUrl( $title->getFullURL(), PROTO_CURRENT );
+ if ( $targetUrl == $request->getFullRequestURL() ) {
+ $message = "Redirect loop detected!\n\n" .
+ "This means the wiki got confused about what page was " .
+ "requested; this sometimes happens when moving a wiki " .
+ "to a new server or changing the server configuration.\n\n";
+
+ if ( $this->config->get( 'UsePathInfo' ) ) {
+ $message .= "The wiki is trying to interpret the page " .
+ "title from the URL path portion (PATH_INFO), which " .
+ "sometimes fails depending on the web server. Try " .
+ "setting \"\$wgUsePathInfo = false;\" in your " .
+ "LocalSettings.php, or check that \$wgArticlePath " .
+ "is correct.";
+ } else {
+ $message .= "Your web server was detected as possibly not " .
+ "supporting URL path components (PATH_INFO) correctly; " .
+ "check your LocalSettings.php for a customized " .
+ "\$wgArticlePath setting and/or toggle \$wgUsePathInfo " .
+ "to true.";
+ }
+ throw new HttpError( 500, $message );
+ }
+ $output->setSquidMaxage( 1200 );
+ $output->redirect( $targetUrl, '301' );
+ return true;
}
/**
diff --git a/tests/phpunit/includes/MediaWikiTest.php b/tests/phpunit/includes/MediaWikiTest.php
new file mode 100644
index 0000000..df94d3e
--- /dev/null
+++ b/tests/phpunit/includes/MediaWikiTest.php
@@ -0,0 +1,157 @@
+<?php
+
+class MediaWikiTest extends MediaWikiTestCase {
+ protected function setUp() {
+ parent::setUp();
+
+ $this->setMwGlobals( array(
+ 'wgServer' => 'http://example.org',
+ 'wgScriptPath' => '/w',
+ 'wgScript' => '/w/index.php',
+ 'wgArticlePath' => '/wiki/$1',
+ 'wgActionPaths' => array(),
+ ) );
+ }
+
+ public static function provideTryNormaliseRedirect() {
+ return array(
+ array(
+ // View: Canonical
+ 'url' => 'http://example.org/wiki/Foo_Bar',
+ 'query' => array(),
+ 'title' => 'Foo_Bar',
+ 'redirect' => false,
+ ),
+ array(
+ // View: Escaped title
+ 'url' => 'http://example.org/wiki/Foo%20Bar',
+ 'query' => array(),
+ 'title' => 'Foo_Bar',
+ 'redirect' => 'http://example.org/wiki/Foo_Bar',
+ ),
+ array(
+ // View: Script path
+ 'url' => 'http://example.org/w/index.php?title=Foo_Bar',
+ 'query' => array( 'title' => 'Foo_Bar' ),
+ 'title' => 'Foo_Bar',
+ 'redirect' => false,
+ ),
+ array(
+ // View: Script path with implicit title from page id
+ 'url' => 'http://example.org/w/index.php?curid=123',
+ 'query' => array( 'curid' => '123' ),
+ 'title' => 'Foo_Bar',
+ 'redirect' => false,
+ ),
+ array(
+ // View: Script path with implicit title from revision id
+ 'url' => 'http://example.org/w/index.php?oldid=123',
+ 'query' => array( 'oldid' => '123' ),
+ 'title' => 'Foo_Bar',
+ 'redirect' => false,
+ ),
+ array(
+ // View: Script path without title
+ 'url' => 'http://example.org/w/index.php',
+ 'query' => array(),
+ 'title' => 'Main_Page',
+ 'redirect' => 'http://example.org/wiki/Main_Page',
+ ),
+ array(
+ // View: Script path with empty title
+ 'url' => 'http://example.org/w/index.php?title=',
+ 'query' => array( 'title' => '' ),
+ 'title' => 'Main_Page',
+ 'redirect' => 'http://example.org/wiki/Main_Page',
+ ),
+ array(
+ // View: Index with escaped title
+ 'url' => 'http://example.org/w/index.php?title=Foo%20Bar',
+ 'query' => array( 'title' => 'Foo Bar' ),
+ 'title' => 'Foo_Bar',
+ 'redirect' => 'http://example.org/wiki/Foo_Bar',
+ ),
+ array(
+ // View: Script path with escaped title
+ 'url' => 'http://example.org/w/?title=Foo_Bar',
+ 'query' => array( 'title' => 'Foo_Bar' ),
+ 'title' => 'Foo_Bar',
+ 'redirect' => false,
+ ),
+ array(
+ // View: Root path with escaped title
+ 'url' => 'http://example.org/?title=Foo_Bar',
+ 'query' => array( 'title' => 'Foo_Bar' ),
+ 'title' => 'Foo_Bar',
+ 'redirect' => false,
+ ),
+ array(
+ // View: Canonical with redundant query
+ 'url' => 'http://example.org/wiki/Foo_Bar?action=view',
+ 'query' => array( 'action' => 'view' ),
+ 'title' => 'Foo_Bar',
+ 'redirect' => false,
+ ),
+ array(
+ // Edit: Canonical view url with action query
+ 'url' => 'http://example.org/wiki/Foo_Bar?action=edit',
+ 'query' => array( 'action' => 'edit' ),
+ 'title' => 'Foo_Bar',
+ 'redirect' => false,
+ ),
+ array(
+ // View: Index with action query
+ 'url' => 'http://example.org/w/index.php?title=Foo_Bar&action=view',
+ 'query' => array( 'title' => 'Foo_Bar', 'action' => 'view' ),
+ 'title' => 'Foo_Bar',
+ 'redirect' => false,
+ ),
+ array(
+ // Edit: Index with action query
+ 'url' => 'http://example.org/w/index.php?title=Foo_Bar&action=edit',
+ 'query' => array( 'title' => 'Foo_Bar', 'action' => 'edit' ),
+ 'title' => 'Foo_Bar',
+ 'redirect' => false,
+ ),
+ );
+ }
+
+ /**
+ * @dataProvider provideTryNormaliseRedirect
+ * @covers MediaWiki::tryNormaliseRedirect
+ */
+ public function testTryNormaliseRedirect( $url, $query, $title, $expectedRedirect = false ) {
+ // Set SERVER because interpolateTitle() doesn't use getRequestURL(),
+ // whereas tryNormaliseRedirect does().
+ $_SERVER['REQUEST_URI'] = $url;
+
+ $req = new FauxRequest( $query );
+ $req->setRequestURL( $url );
+ // This adds a virtual 'title' query parameter. Normally called from Setup.php
+ $req->interpolateTitle();
+
+ $titleObj = Title::newFromText( $title );
+
+ // Set global context since some involved code paths don't yet have context
+ $context = RequestContext::getMain();
+ $context->setRequest( $req );
+ $context->setTitle( $titleObj );
+
+ $mw = new MediaWiki( $context );
+
+ $method = new ReflectionMethod( $mw, 'tryNormaliseRedirect' );
+ $method->setAccessible( true );
+ $ret = $method->invoke( $mw, $titleObj );
+
+ $this->assertEquals(
+ $expectedRedirect !== false,
+ $ret,
+ 'Return true only when redirecting'
+ );
+
+ $this->assertEquals(
+ $expectedRedirect ?: '',
+ $context->getOutput()->getRedirect()
+ );
+ }
+}
--
2.6.4
From 2e002007ff6b24644b23f6e5fdb6fce373999e23 Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Mon, 24 Aug 2015 13:00:23 -0700
Subject: [PATCH 08/10] Fixed some doc errors in tryNormaliseRedirect()
Change-Id: I8f9397d05de1c0bae33497d1f9e3146939599380
---
includes/MediaWiki.php | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php
index e06877f..0ccdcd9 100644
--- a/includes/MediaWiki.php
+++ b/includes/MediaWiki.php
@@ -269,9 +269,11 @@ class MediaWiki {
* /w/index.php?title= -> /wiki/Main
* - Don't redirect anything with query parameters other than 'title' or 'action=view'.
*
+ * @param Title $title
* @return bool True if a redirect was set.
+ * @throws HttpError
*/
- private function tryNormaliseRedirect( $title ) {
+ private function tryNormaliseRedirect( Title $title ) {
$request = $this->context->getRequest();
$output = $this->context->getOutput();
--
2.6.4
From 5bea2b8aad66559513d1dce6e0c1e318209ac843 Mon Sep 17 00:00:00 2001
From: csteipp <csteipp@wikimedia.org>
Date: Mon, 5 Oct 2015 16:58:42 -0700
Subject: [PATCH 09/10] SECURITY: Make Special:MyPage and friends fake redirect
to prevent info leak
This prevents a malicious person from using external resources on their
website to cause the victim's web browser to load
Special:MyPage -> User:Username, and then looking it up in the page hit
statistics in order to correlate IPs from the malicious person's server
log, with usernames on wiki.
This feature can be disabled with $wgHideIdentifiableRedirects.
Bug: T109724
Change-Id: Ia0e742dc92c77af4832174dfa24c6dcaa6ee80e9
Signed-off-by: Chad Horohoe <chadh@wikimedia.org>
---
includes/DefaultSettings.php | 6 ++++
includes/MediaWiki.php | 44 ++++++++++++++++++++----
includes/specialpage/RedirectSpecialPage.php | 12 +++++++
includes/specials/SpecialMyLanguage.php | 11 ++++++
includes/specials/SpecialMyRedirectPages.php | 50 ++++++++++++++++++++++++++++
5 files changed, 117 insertions(+), 6 deletions(-)
diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 3f6f0ae..e58c051 100644
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -4466,6 +4466,12 @@ $wgWhitelistReadRegexp = false;
$wgEmailConfirmToEdit = false;
/**
+ * Should MediaWiki attempt to protect user's privacy when doing redirects?
+ * Keep this true if access counts to articles are made public.
+ */
+$wgHideIdentifiableRedirects = true;
+
+/**
* Permission keys given to users in each group.
*
* This is an array where the keys are all groups and each value is an
diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php
index 0ccdcd9..fda3a73 100644
--- a/includes/MediaWiki.php
+++ b/includes/MediaWiki.php
@@ -37,6 +37,11 @@ class MediaWiki {
private $config;
/**
+ * @var String Cache what action this request is
+ */
+ private $action;
+
+ /**
* @param IContextSource|null $context
*/
public function __construct( IContextSource $context = null ) {
@@ -133,13 +138,11 @@ class MediaWiki {
* @return string Action
*/
public function getAction() {
- static $action = null;
-
- if ( $action === null ) {
- $action = Action::getActionName( $this->context );
+ if ( $this->action === null ) {
+ $this->action = Action::getActionName( $this->context );
}
- return $action;
+ return $this->action;
}
/**
@@ -230,8 +233,37 @@ class MediaWiki {
// Handle any other redirects.
// Redirect loops, titleless URL, $wgUsePathInfo URLs, and URLs with a variant
} elseif ( !$this->tryNormaliseRedirect( $title ) ) {
+ // Prevent information leak via Special:MyPage et al (T109724)
+ if ( $title->isSpecialPage() ) {
+ $specialPage = SpecialPageFactory::getPage( $title->getDBKey() );
+ if ( $specialPage instanceof RedirectSpecialPage
+ && $this->config->get( 'HideIdentifiableRedirects' )
+ && $specialPage->personallyIdentifiableTarget()
+ ) {
+ list( , $subpage ) = SpecialPageFactory::resolveAlias( $title->getDBKey() );
+ $target = $specialPage->getRedirect( $subpage );
+ // target can also be true. We let that case fall through to normal processing.
+ if ( $target instanceof Title ) {
+ $query = $specialPage->getRedirectQuery() ?: array();
+ $request = new DerivativeRequest( $this->context->getRequest(), $query );
+ $request->setRequestURL( $this->context->getRequest()->getRequestURL() );
+ $this->context->setRequest( $request );
+ // Do not varnish cache these. May vary even for anons
+ $this->context->getOutput()->lowerCdnMaxage( 0 );
+ $this->context->setTitle( $target );
+ $wgTitle = $target;
+ // Reset action type cache. (Special pages have only view)
+ $this->action = null;
+ $title = $target;
+ $output->addJsConfigVars( array(
+ 'wgInternalRedirectTargetUrl' => $target->getFullURL(),
+ ) );
+ $output->addModules( 'mediawiki.action.view.redirect' );
+ }
+ }
+ }
- // Special pages
+ // Special pages ($title may have changed since if statement above)
if ( NS_SPECIAL == $title->getNamespace() ) {
// Actions that need to be made when we have a special pages
SpecialPageFactory::executePath( $title, $this->context );
diff --git a/includes/specialpage/RedirectSpecialPage.php b/includes/specialpage/RedirectSpecialPage.php
index 4226ee0..a3bfa1b 100644
--- a/includes/specialpage/RedirectSpecialPage.php
+++ b/includes/specialpage/RedirectSpecialPage.php
@@ -89,6 +89,18 @@ abstract class RedirectSpecialPage extends UnlistedSpecialPage {
? $params
: false;
}
+
+ /**
+ * Indicate if the target of this redirect can be used to identify
+ * a particular user of this wiki (e.g., if the redirect is to the
+ * user page of a User). See T109724.
+ *
+ * @since 1.27
+ * @return bool
+ */
+ public function personallyIdentifiableTarget() {
+ return false;
+ }
}
/**
diff --git a/includes/specials/SpecialMyLanguage.php b/includes/specials/SpecialMyLanguage.php
index 71b1893..497cb7e 100644
--- a/includes/specials/SpecialMyLanguage.php
+++ b/includes/specials/SpecialMyLanguage.php
@@ -90,4 +90,15 @@ class SpecialMyLanguage extends RedirectSpecialArticle {
return $base;
}
}
+
+ /**
+ * Target can identify a specific user's language preference.
+ *
+ * @see T109724
+ * @since 1.27
+ * @return bool
+ */
+ public function personallyIdentifiableTarget() {
+ return true;
+ }
}
diff --git a/includes/specials/SpecialMyRedirectPages.php b/includes/specials/SpecialMyRedirectPages.php
index 9b8d52b..65ac864 100644
--- a/includes/specials/SpecialMyRedirectPages.php
+++ b/includes/specials/SpecialMyRedirectPages.php
@@ -41,6 +41,16 @@ class SpecialMypage extends RedirectSpecialArticle {
return Title::makeTitle( NS_USER, $this->getUser()->getName() );
}
}
+
+ /**
+ * Target identifies a specific User. See T109724.
+ *
+ * @since 1.27
+ * @return bool
+ */
+ public function personallyIdentifiableTarget() {
+ return true;
+ }
}
/**
@@ -60,6 +70,16 @@ class SpecialMytalk extends RedirectSpecialArticle {
return Title::makeTitle( NS_USER_TALK, $this->getUser()->getName() );
}
}
+
+ /**
+ * Target identifies a specific User. See T109724.
+ *
+ * @since 1.27
+ * @return bool
+ */
+ public function personallyIdentifiableTarget() {
+ return true;
+ }
}
/**
@@ -77,6 +97,16 @@ class SpecialMycontributions extends RedirectSpecialPage {
function getRedirect( $subpage ) {
return SpecialPage::getTitleFor( 'Contributions', $this->getUser()->getName() );
}
+
+ /**
+ * Target identifies a specific User. See T109724.
+ *
+ * @since 1.27
+ * @return bool
+ */
+ public function personallyIdentifiableTarget() {
+ return true;
+ }
}
/**
@@ -93,6 +123,16 @@ class SpecialMyuploads extends RedirectSpecialPage {
function getRedirect( $subpage ) {
return SpecialPage::getTitleFor( 'Listfiles', $this->getUser()->getName() );
}
+
+ /**
+ * Target identifies a specific User. See T109724.
+ *
+ * @since 1.27
+ * @return bool
+ */
+ public function personallyIdentifiableTarget() {
+ return true;
+ }
}
/**
@@ -111,4 +151,14 @@ class SpecialAllMyUploads extends RedirectSpecialPage {
return SpecialPage::getTitleFor( 'Listfiles', $this->getUser()->getName() );
}
+
+ /**
+ * Target identifies a specific User. See T109724.
+ *
+ * @since 1.27
+ * @return bool
+ */
+ public function personallyIdentifiableTarget() {
+ return true;
+ }
}
--
2.6.4
From bea57788f35180bd85c65e62bb29c38a9afa6b4b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bartosz=20Dziewo=C5=84ski?= <matma.rex@gmail.com>
Date: Thu, 19 Nov 2015 17:13:13 -0500
Subject: [PATCH 10/10] Add $query to JavaScript redirect info
Bug: T109724
Change-Id: I57a8f75067365d3da6388d2f8f7fe95ed5e6f310
Signed-off-by: Chad Horohoe <chadh@wikimedia.org>
---
includes/MediaWiki.php | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php
index fda3a73..b828ad3 100644
--- a/includes/MediaWiki.php
+++ b/includes/MediaWiki.php
@@ -256,7 +256,7 @@ class MediaWiki {
$this->action = null;
$title = $target;
$output->addJsConfigVars( array(
- 'wgInternalRedirectTargetUrl' => $target->getFullURL(),
+ 'wgInternalRedirectTargetUrl' => $target->getFullURL( $query ),
) );
$output->addModules( 'mediawiki.action.view.redirect' );
}
--
2.6.4

Event Timeline