Page MenuHomePhabricator

MediaWiki 1.28.1/1.27.2/1.23.16 security release
Closed, ResolvedPublic

Description

MW Versions: 1.28.1/1.27.2/1.23.16.
Previous release work: T133070

Core

Maniphest IDCVE IDREL1_23REL1_27REL1_28REL1_29/master
T68404CVE-2017-0371DoneDoneDoneDone
T156184 (part 1)CVE-2017-0368DoneDoneDoneDone
T109140/T122209CVE-2017-0363 / CVE-2017-0364
T144845CVE-2017-0365
T125177CVE-2017-0361
T150044CVE-2017-0362
T156184 (part 2)CVE-2017-0368
T151735CVE-2017-0366
T161453CVE-2017-0367n/a (introduced in REL1_27)
T48143CVE-2017-0370
T108138CVE-2017-0369
Patch tarballs

Extensions

Like usual, please don't add an extension if we don't bundle those -- see the list

Maniphest IDCVE IDREL1_23REL1_27REL1_28REL1_29/master
T158689CVE-2017-0372n/a

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Reedy updated the task description. (Show Details)Mar 17 2017, 2:08 AM
Reedy added a comment.Mar 19 2017, 9:52 PM

So, a REL1_23 patch, and unapplied (to original location) changes below...

  • ChangeCredentials change ported to SpecialChangePassword
  • PageLanguage came in REL1_24, so we don't need it
  • SpecialSearch - SpecialSearchGoResult hook came in in REL1_27, so not necessary either
  • LoginHelper change ported to SpecialUserlogin
diff --git a/includes/specials/SpecialChangeCredentials.php b/includes/specials/SpecialChangeCredentials.php
index 9bf25ae..b2ffdc6 100644
--- a/includes/specials/SpecialChangeCredentials.php
+++ b/includes/specials/SpecialChangeCredentials.php
@@ -243,7 +243,7 @@ class SpecialChangeCredentials extends AuthManagerSpecialPage {
 		}
 
 		$title = Title::newFromText( $returnTo );
-		return $title->getFullURL( $returnToQuery );
+		return $title->getFullUrlForRedirect( $returnToQuery );
 	}
 
 	protected function getRequestBlacklist() {
diff --git a/includes/specials/SpecialPageLanguage.php b/includes/specials/SpecialPageLanguage.php
index 5322a04..0035829 100644
--- a/includes/specials/SpecialPageLanguage.php
+++ b/includes/specials/SpecialPageLanguage.php
@@ -141,7 +141,7 @@ class SpecialPageLanguage extends FormSpecialPage {
 		);
 
 		// Url to redirect to after the operation
-		$this->goToUrl = $title->getFullURL();
+		$this->goToUrl = $title->getFullUrlForRedirect();
 
 		// Check if user wants to use default language
 		if ( $data['selectoptions'] == 1 ) {
diff --git a/includes/specials/SpecialSearch.php b/includes/specials/SpecialSearch.php
index d474ba5..9ba948c 100644
--- a/includes/specials/SpecialSearch.php
+++ b/includes/specials/SpecialSearch.php
@@ -218,7 +218,7 @@ class SpecialSearch extends SpecialPage {
 			Hooks::run( 'SpecialSearchGoResult', [ $term, $title, &$url ] )
 		) {
 			if ( $url === null ) {
-				$url = $title->getFullURL();
+				$url = $title->getFullUrlForRedirect();
 			}
 			$this->getOutput()->redirect( $url );
 
diff --git a/includes/specials/helpers/LoginHelper.php b/includes/specials/helpers/LoginHelper.php
index f853f41..cfcbf65 100644
--- a/includes/specials/helpers/LoginHelper.php
+++ b/includes/specials/helpers/LoginHelper.php
@@ -89,7 +89,7 @@ class LoginHelper extends ContextSource {
 		}
 
 		if ( $type === 'successredirect' ) {
-			$redirectUrl = $returnToTitle->getFullURL( $returnToQuery, false, $proto );
+			$redirectUrl = $returnToTitle->getFullUrlForRedirect( $returnToQuery, $proto );
 			$this->getOutput()->redirect( $redirectUrl );
 		} else {
 			$this->getOutput()->addReturnTo( $returnToTitle, $returnToQuery, null, $options );
Reedy updated the task description. (Show Details)Mar 19 2017, 9:52 PM
Reedy added a comment.EditedMar 19 2017, 10:24 PM

Bad patch

Reedy updated the task description. (Show Details)Mar 19 2017, 10:24 PM
Reedy updated the task description. (Show Details)Mar 19 2017, 10:29 PM

2nd time lucky

Reedy updated the task description. (Show Details)Mar 19 2017, 10:42 PM

Reedy updated the task description. (Show Details)Mar 19 2017, 10:56 PM

So that's a complete set of patches for the backports....

Probably need some release-notes adding at the end as some patches don't seem to have them, so weren't backported

Guess I need to get ontop of the CVE's too

Then just seeing if/what applies to master...

Write some announcement emails, and get this shipped. And hopefully see the back of REL1_23

All patches currently apply to master where HEAD is d8852217e2d8b7d2e5fc2ec9b79196f47ea0836a

reedy@ubuntu64-web-esxi:~/git/mediawiki/core$ git am 01-T109140-master.patch
Applying: SECURITY: Do not directly redirect to interwikis, but use splash page           
reedy@ubuntu64-web-esxi:~/git/mediawiki/core$ git am T144845-master.patch
Applying: SECURITY: XSS in search if $wgAdvancedSearchHighlighting = true;
reedy@ubuntu64-web-esxi:~/git/mediawiki/core$ git am 03-T125177-master.patch
Applying: SECURITY: API: Don't log "sensitive" parameters
reedy@ubuntu64-web-esxi:~/git/mediawiki/core$ git am 04-T150044-master.patch
Applying: SECURITY: SpecialWatchlist: Check CSRF token when using "Mark all pages visited"
reedy@ubuntu64-web-esxi:~/git/mediawiki/core$
Reedy added a comment.EditedMar 19 2017, 11:21 PM

For consistency/due diligence/best practices, review of current core patches on tin:

reedy@tin:/srv/patches/1.29.0-wmf.16/core$ ls -al
total 44
drwxrwxr-x 2 twentyafterfour wikidev  4096 Mar 13 21:30 .
drwxrwxr-x 4 twentyafterfour wikidev  4096 Feb 28 18:06 ..
-rw-rw-r-- 1 twentyafterfour wikidev 14245 Feb 28 18:06 01-T109140.patch
-rw-r--r-- 1 twentyafterfour wikidev  1679 Feb 28 18:06 02-T127114-master_1.28wmf2.patch
-rw-rw-r-- 1 twentyafterfour wikidev  6903 Feb 28 18:06 03-T125177.patch
-rw-rw-r-- 1 twentyafterfour wikidev  1505 Feb 28 18:06 04-T150044.patch
-rw-rw-r-- 1 twentyafterfour wikidev  1318 Mar 13 21:30 05-T160266.patch
reedy@tin:/srv/patches/1.29.0-wmf.16/core$

5 patches on master

1 is a prevention, but maybe not the best fix for T160266. It'd be nice if we could include it in the release, if ready in time, but if it's not ready, it's not ready.

Patch for T127114 is still staged on tin (To be applied). Can we drop it yet per Gergo's comments on the bug? Makes things cleaner

T144845 isn't applicable for WMF cluster, so not deployed currently

Only extension patch is SyntaxHighlight_GeSHi, patch is above

cf target bug

If we don't need these anymore, we should drop them from prod
I see https://gerrit.wikimedia.org/r/#/c/270669/ on master, but it doesn't seem it was ever backported.... Do we want it backported?
T127114 says "done"...
Gerrit says

Branches	REL1_27, REL1_28, fundraising/REL1_27, master, sandbox/twentyafterfour/group0, wmf/1.29.0-wmf.10, wmf/1.29.0-wmf.11, wmf/1.29.0-wmf.12, wmf/1.29.0-wmf.13, wmf/1.29.0-wmf.14, wmf/1.29.0-wmf.15, wmf/1.29.0-wmf.16, wmf/1.29.0-wmf.2, wmf/1.29.0-wmf.3, wmf/1.29.0-wmf.4, wmf/1.29.0-wmf.5, wmf/1.29.0-wmf.6, wmf/1.29.0-wmf.7, wmf/1.29.0-wmf.8
Tags	1.27.0, 1.27.0-rc.0, 1.27.0-rc.1, 1.27.1, 1.28.0, 1.28.0-rc.0, 1.28.0-rc.1

So we just need to do REL1_23?
Has this been publicised?

Why is T127114 listed on here? It might need to be backported to REL1_23... But other than that, it's been merged far enough back to be in 1.27.0 anyway...

Reedy updated the task description. (Show Details)Mar 19 2017, 11:55 PM

Seems I'm repeating myself.

For REL1_23 https://github.com/wikimedia/mediawiki/commit/3c00b14e7df7bc761646cd016516d21e892c1fe2 made it into 1.23.14/1.23.15

So, the question is why this is tagged against this bug? Anyone know?

For T127114, I note that that branches already have this patch... But no mention of it in 1.27.1, 1.26.4, 1.23.15? For 1.23.. It was in 1.23.14 https://lists.wikimedia.org/pipermail/mediawiki-announce/2016-August/000195.html
REL1_23: https://github.com/wikimedia/mediawiki/commit/3c00b14e7df7bc761646cd016516d21e892c1fe2
REL1_27: https://github.com/wikimedia/mediawiki/commit/1b93286162f23e1633607293e4dbd878d28b54a7
So why are these patches listed for this release? Because it wasn't apparently publicised?

Reedy updated the task description. (Show Details)Mar 19 2017, 11:58 PM

Other subtasks listed... But no patches up at the top. Are we wanting to include them?

Reedy created subtask Restricted Task.Mar 20 2017, 12:34 AM
Bawolff updated the task description. (Show Details)Mar 27 2017, 1:44 PM
Reedy added a comment.Mar 28 2017, 8:23 PM

Tentatively suggesting friday as the cut off for new tasks to be added (ignoring any UNBREAK NOW that comes in as per usual)

demon added a subscriber: demon.Mar 30 2017, 5:52 PM
Reedy updated the task description. (Show Details)Mar 30 2017, 5:55 PM
Reedy added a comment.Mar 30 2017, 6:04 PM

T48143 too.

Depending on the decision, they should be removed as children from this task

Reedy updated the task description. (Show Details)Mar 30 2017, 10:10 PM

T48143 and T108138 both now have a full complement of patches. But untested

In T140591#3113328, @Reedy wrote:

For consistency/due diligence/best practices, review of current core patches on tin:

reedy@tin:/srv/patches/1.29.0-wmf.16/core$ ls -al
total 44
drwxrwxr-x 2 twentyafterfour wikidev  4096 Mar 13 21:30 .
drwxrwxr-x 4 twentyafterfour wikidev  4096 Feb 28 18:06 ..
-rw-rw-r-- 1 twentyafterfour wikidev 14245 Feb 28 18:06 01-T109140.patch
-rw-r--r-- 1 twentyafterfour wikidev  1679 Feb 28 18:06 02-T127114-master_1.28wmf2.patch
-rw-rw-r-- 1 twentyafterfour wikidev  6903 Feb 28 18:06 03-T125177.patch
-rw-rw-r-- 1 twentyafterfour wikidev  1505 Feb 28 18:06 04-T150044.patch
-rw-rw-r-- 1 twentyafterfour wikidev  1318 Mar 13 21:30 05-T160266.patch
reedy@tin:/srv/patches/1.29.0-wmf.16/core$

5 patches on master
1 is a prevention, but maybe not the best fix for T160266. It'd be nice if we could include it in the release, if ready in time, but if it's not ready, it's not ready.

Otoh it might make sense to do it as a private part of the fix for T156184

Need to make decisions about these last few... We wanted to use today mostly as a cut off point, looking to get the release done next Thursday....

Reedy updated the task description. (Show Details)Apr 1 2017, 4:23 PM
Reedy updated the task description. (Show Details)Apr 1 2017, 4:26 PM
Reedy added a comment.Apr 1 2017, 4:37 PM

@Bawolff What's the other "secret" patch to be associated with T156184 ?

Reedy updated the task description. (Show Details)Apr 1 2017, 9:08 PM
Bawolff added a comment.EditedApr 1 2017, 9:51 PM

@Bawolff What's the other "secret" patch to be associated with T156184 ?

The one at T160266#3117039

EDIT I meant the other patch - https://phabricator.wikimedia.org/T160266#3096811 . The one that's deployed on cluster

Bawolff updated the task description. (Show Details)Apr 1 2017, 10:15 PM
Reedy updated the task description. (Show Details)Apr 2 2017, 3:12 PM
Reedy added a comment.Apr 2 2017, 5:42 PM

Rather than trying to update the table above, and also to make life easier for the actual release, here are some tars of all the patches! Including the bump/release note finalisations patch

1.28.1


Reedy updated the task description. (Show Details)Apr 2 2017, 5:44 PM
Reedy added a comment.Apr 2 2017, 6:37 PM

1.27.2


Reedy updated the task description. (Show Details)Apr 2 2017, 6:37 PM
Reedy added a comment.Apr 2 2017, 7:10 PM

So, as we've not decided about T125177. I've made 2 sets of patches for REL1_23, one has https://gerrit.wikimedia.org/r/#/c/336838/ at the top (and as such, needs merging first if we go down that route), and one without it.

I want to get this resolved before we get that far to do the release, but doing the prep now for ease


With 336838


Without 336838

Reedy updated the task description. (Show Details)Apr 2 2017, 7:10 PM
Reedy updated the task description. (Show Details)Apr 2 2017, 10:22 PM
Reedy added a comment.Apr 2 2017, 11:45 PM


Reedy updated the task description. (Show Details)Apr 2 2017, 11:45 PM
Grunny added a subscriber: Grunny.Apr 4 2017, 1:40 PM

I believe the patch for MW 1.23 related to T48143 will break as it adds the call to parser::normalizeLinkUrl but the Parser::normalizeLinkUrl method does not exist in MW 1.23, as it is still using Parser::replaceUnusualEscapes instead.

Reedy added a comment.Apr 4 2017, 1:48 PM

Looks like you're right, thanks @Grunny

Of course, helpfully, https://github.com/wikimedia/mediawiki/blob/4f43a9608fd4af27cce666c94caabb8106d50cc4/includes/parser/Parser.php#L1980 doesn't have a @since tag on it...

@Bawolff Is swapping to replaceUnusualEscapes in REL1_23 enough, or do we need to backport some/all of normalizeLinkUrl from a newer version?

Reedy added a comment.Apr 4 2017, 2:47 PM

[15:33:54] <bawolff> yeah, looking at e2c9d4df, replaceUnusualEscapes is good enough for our purposes

I'll get the patch updated and re-tarred

Grunny added a comment.Apr 4 2017, 4:36 PM

For the MW 1.23 patch for T108138, it looks like $user is undefined in the Title::userCan call as well, as it currently repeatedly calls $this->getUser() rather than storing it in a $user variable.

Reedy added a comment.Apr 4 2017, 4:47 PM

For the MW 1.23 patch for T108138, it looks like $user is undefined in the Title::userCan call as well, as it currently repeatedly calls $this->getUser() rather than storing it in a $user variable.

The latter isn't great, but isn't a major issue (MW won't be creating new user objects every time). Will fix it up at the same time

Reedy added a comment.Apr 4 2017, 5:12 PM

Updated, thanks @Grunny

Reedy updated the task description. (Show Details)Apr 4 2017, 5:13 PM
Grunny added a comment.Apr 5 2017, 9:08 AM

Thanks, @Reedy! Found one more issue in the patches for MW 1.23. In the patch for T108138, the calls to getUserPermissionsErrorsInternal are passing the undefined variable $rigor as MW 1.23 is still using the boolean $doExpensiveQueries instead. The call to Title::userCan should also probably use a boolean true instead of 'secure' for consistency.

Reedy added a comment.Apr 5 2017, 5:27 PM

Reedy updated the task description. (Show Details)Apr 5 2017, 5:28 PM

Thanks again @Grunny

Are Wikia going to stop using such an ancient MW anytime soon so they don't care about 1.23? ;)

Reedy updated the task description. (Show Details)Apr 5 2017, 9:09 PM
Reedy added a comment.Apr 5 2017, 11:06 PM

API changes are gonna land, https://gerrit.wikimedia.org/r/#/c/336838/ is merged in public

Reedy updated the task description. (Show Details)Apr 5 2017, 11:07 PM
Reedy closed subtask Restricted Task as Resolved.Apr 6 2017, 2:02 PM
Reedy updated the task description. (Show Details)Apr 6 2017, 3:51 PM
Reedy added a comment.Apr 6 2017, 6:26 PM

Rebase for patch merged into REL1_28

Reedy updated the task description. (Show Details)Apr 6 2017, 6:27 PM
Reedy claimed this task.Apr 6 2017, 8:52 PM
Reedy edited projects, added MediaWiki-General, Security-Team; removed Security.
Reedy changed the visibility from "Custom Policy" to "Public (No Login Required)".
Restricted Application added a project: Security. · View Herald TranscriptApr 6 2017, 8:52 PM
Reedy closed this task as Resolved.Apr 6 2017, 8:53 PM
sbassett moved this task from Backlog to Done on the Security-Team board.Jun 11 2019, 6:34 PM