Page MenuHomePhabricator

MediaWiki 1.28.1/1.27.2/1.23.16 security release
Closed, ResolvedPublic

Assigned To
Authored By
Bawolff
Jul 18 2016, 7:47 AM
Referenced Files
F7315873: 1.28.1v2.tar
Apr 6 2017, 6:26 PM
F7280208: 1.23.16v4.tar
Apr 5 2017, 11:06 PM
F7270342: 1.23.16v3-withoutapisecurity.tar
Apr 5 2017, 5:27 PM
F7270341: 1.23.16v3-withapisecurity.tar
Apr 5 2017, 5:27 PM
F7214130: 1.23.16v2-withoutapisecurity.tar
Apr 4 2017, 5:12 PM
F7214128: 1.23.16v2-withapisecurity.tar
Apr 4 2017, 5:12 PM
F7153973: 0006-SECURITY-Whitelist-DTD-declaration-in-SVG.patch
Apr 2 2017, 11:45 PM

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

2nd time lucky

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$

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...

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?

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

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

T48143 too.

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

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....

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

@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

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


1.27.2


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


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.

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?

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

I'll get the patch updated and re-tarred

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.

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

Updated, thanks @Grunny

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.

Thanks again @Grunny

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

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

Reedy closed subtask Restricted Task as Resolved.Apr 6 2017, 2:02 PM

Rebase for patch merged into REL1_28

Reedy edited projects, added MediaWiki-General, Security-Team; removed acl*security.
Reedy changed the visibility from "Custom Policy" to "Public (No Login Required)".