It looks this was an API-based edit.

[urbanecm@mwlog1001 /srv/mw-log]$ grep 82.102.26.202 api.log
[snip]
2019-08-03 15:43:08 [XUWrjApAAEsAAJ55r7kAAACH] mw1280 eswiki 1.34.0-wmf.16 api INFO: API POST 82.102.26.202 82.102.26.202 T=833ms action=edit format=json title=Rosal%C3%ADa%20Vila section=0 text=%7B%7BFicha%20de%20artista%20musical%0A%7Cnombre%20%20%20%20%20%20%20%20%20%20%20%20%3D%20Rosal%C3%ADa%0A%7Cimagen%20%20%20%20%20%20%20%20%20%20%20%20%3D%20Rosalia%202019-portrait.jpg%0A%7Csubt%C3%ADtulo%20%20%20%20%20%20%20%20%20%3D%20Rosal%C3%ADa%20en%20los%20%5B%5BPremios%20Goya%5D%5D%20de%202019.%0A%7Ctama%C3%B1o%20%20%20%20%20%20%20%20%20%20%20%20%3D%20250px%0A%7Cfondo%20%20%20%20%20%20%20%20%20%20%20%20%20%3D%20solista%0A%7Cnombre%20real%20%20%20%20%20%20%20%3D%20Rosal%C3%ADa%20V[...] summary=Eliminaci%C3%B3n%20de%20carga%20ideol%C3%B3gica. basetimestamp=2019-07-31T18:50:43Z starttimestamp=2019-07-31T18:50:43Z token=[redacted]
[snip]

I've been discussing with @Urbanecm on IRC with this.

My guess is that this might be some sort of caching issue. 82.102.16.0/20 was globally blocked as part of a batch of global blocks which were all issued using a Toolforge tool. Maybe the database, given that those were issued at high speed, failed to load them properly or something like that?

Edit: this is not the reason. See updated description for details.

Do I understand GlobalBlocking::getGlobalBlockingBlock()'s $block = $dbr->selectRow( 'globalblocks', self::selectFields(), $conds, __METHOD__ ); correctly? I feel it means "select the first block in the database", which would usually mean the oldest one. Maybe we should add a method like DatabaseBlock::chooseMostSpecificBlock() to GlobalBlocking::getGlobalBlockingBlock() and select the most specific block. That would solve this issue, since the smaller block would be used, instead of the /19 one.

In T229731#5389836, @Urbanecm wrote:

Do I understand GlobalBlocking::getGlobalBlockingBlock()'s $block = $dbr->selectRow( 'globalblocks', self::selectFields(), $conds, __METHOD__ ); correctly? I feel it means "select the first block in the database", which would usually mean the oldest one. Maybe we should add a method like DatabaseBlock::chooseMostSpecificBlock() to GlobalBlocking::getGlobalBlockingBlock() and select the most specific block. That would solve this issue, since the smaller block would be used, instead of the /19 one.

selectRow will indeed only return one row. Whether it's the oldest is indeterminate when a sort order isn't provided

Rebased. Anyone to review?

Second attempt...

...?

...?

Hi @Urbanecm

Regretfully, this ticket was outside of the team workflow thus your requests for review were not seen or acted upon by the Security Team in a timely fashion. We apologize for the confusion, as we are working hard to improve and evolve our processes. For future reference, tagging Security is not considered a request for service from the Security Team and following our SOP will ensure our prompt attention to your needs. Thank you - and again, we apologize for the confusion. We are adding our Security-Team tag and will take a look during our next clinic meeting.

Cheers

Third time's the charm.

@Jcross Thanks for the explanation - was there any update with this patch?

@Urbanecm - Sorry for the delay on this. We should hopefully be able to get it reviewed and deployed sometime next week.

@Urbanecm - I've reviewed the patch and can +1 for now as it seems sane, though I didn't get to test it as completely as I'd like. I'm thinking it can probably go out during this coming Monday's sec deployment window. Would you be around to help test?

In T229731#5908440, @sbassett wrote:

@Urbanecm - I've reviewed the patch and can +1 for now as it seems sane, though I didn't get to test it as completely as I'd like. I'm thinking it can probably go out during this coming Monday's sec deployment window. Would you be around to help test?

Unfortunately, I'm not a steward and I cannot place global blocks. To fully test, we'd need some ranges with IPs I'm able to access globally blocked. Maybe @MarcoAurelio could help us place the blocks? Or, we can also wait for the end of SE2020 :-).

In T229731#5909410, @Urbanecm wrote:

Unfortunately, I'm not a steward and I cannot place global blocks. To fully test, we'd need some ranges with IPs I'm able to access globally blocked. Maybe @MarcoAurelio could help us place the blocks? Or, we can also wait for the end of SE2020 :-).

It depends on the urgency. I could probably coordinate with both of you to attempt to place some global blocks and see if this works or you can wait until SE is over and do it yourself (and therefore save my fees :-P ). As you wish :-)

Given this waited in the queue for months, a week shouldn't hurt us much. @sbassett Would Mar 2 sec window be okay with you?

In T229731#5910210, @Urbanecm wrote:

Given this waited in the queue for months, a week shouldn't hurt us much. @sbassett Would Mar 2 sec window be okay with you?

@Urbanecm @MarcoAurelio -

Should be fine. I know the weekly security deployment window (UTC 22:00–00:00) isn't very Euro-friendly - would you prefer to try to add this to the European SWAT on March 2nd? I'm happy to help test/deploy at either time.

In T229731#5913887, @sbassett wrote:

Should be fine. I know the weekly security deployment window (UTC 22:00–00:00) isn't very Euro-friendly - would you prefer to try to add this to the European SWAT on March 2nd? I'm happy to help test/deploy at either time.

@Urbanecm @MarcoAurelio - does the above work for Monday, March 2nd?

EU SWAT would be better, but I can make it if needed.

@Urbanecm - do we need to get this on the deployment schedule or no?

@MarcoAurelio - does 12:00–13:00 UTC March 2nd work for you?

In T229731#5928176, @sbassett wrote:

@MarcoAurelio - does 12:00–13:00 UTC March 2nd work for you?

I can't make any promises. However @Urbanecm is now a steward and can access the production global blocking interface to test if needed.

recent changes made me do one other rebase...

13:58 <Urbanecm> !log Deploy security fix for T229731

This is now live, and per my test.wikipedia testing, it works.

@Urbanecm - Thanks for patching and deploying this! I've added this to the tracking bug for the next security release: T240393. So let's keep this task private and hold off on the backports/CVE for a bit.

In T229731#5933865, @sbassett wrote:

@Urbanecm - Thanks for patching and deploying this! I've added this to the tracking bug for the next security release: T240393. So let's keep this task private and hold off on the backports/CVE for a bit.

Noting GlobalBlocking is not bundled, and it's not a core patch...

In T229731#5956124, @Reedy wrote:

Noting GlobalBlocking is not bundled, and it's not a core patch...

Ah, yes. So... I guess if this patch seems stable in prod, we can make this task public and start the backports.

In T229731#5956225, @sbassett wrote:

In T229731#5956124, @Reedy wrote:

Noting GlobalBlocking is not bundled, and it's not a core patch...

Ah, yes. So... I guess if this patch seems stable in prod, we can make this task public and start the backports.

Yeah, WFM. If we get the patch into master now ish, it can go out with the train in a few hours

We still want a CVE for this one though, right?

Change 578566 had a related patch set uploaded (by SBassett; owner: Urbanecm):
[mediawiki/extensions/GlobalBlocking@master] SECURITY: Apply most specific global block and make sure applied block matches block showed on Special:Contributions

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

Change 578566 merged by SBassett:
[mediawiki/extensions/GlobalBlocking@master] SECURITY: Apply most specific global block and make sure applied block matches block showed on Special:Contributions

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

I've submitted the CVE request for this issue - I'll update this task and T240400 once I have the ID. Gerrit couldn't seem to handle simple picks for the backports to 1.31, 1.33 and 1.34, so those will likely involve a bit more work, if they're even feasible.

In T229731#5965728, @sbassett wrote:

I've submitted the CVE request for this issue - I'll update this task and T240400 once I have the ID. Gerrit couldn't seem to handle simple picks for the backports to 1.31, 1.33 and 1.34, so those will likely involve a bit more work, if they're even feasible.

Basically if there's any conflicts, gerrit says no. jgit sucks

Change 579427 had a related patch set uploaded (by Reedy; owner: Urbanecm):
[mediawiki/extensions/GlobalBlocking@REL1_34] SECURITY: Apply most specific global block and make sure applied block matches block showed on Special:Contributions

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

Change 579428 had a related patch set uploaded (by Reedy; owner: Urbanecm):
[mediawiki/extensions/GlobalBlocking@REL1_33] SECURITY: Apply most specific global block and make sure applied block matches block showed on Special:Contributions

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

Change 579429 had a related patch set uploaded (by Reedy; owner: Urbanecm):
[mediawiki/extensions/GlobalBlocking@REL1_31] SECURITY: Apply most specific global block and make sure applied block matches block showed on Special:Contributions

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

Change 579431 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/GlobalBlocking@master] Use IPUtils instead of deprecated IP class

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

Change 579427 merged by jenkins-bot:
[mediawiki/extensions/GlobalBlocking@REL1_34] SECURITY: Apply most specific global block and make sure applied block matches block showed on Special:Contributions

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

Change 579431 merged by jenkins-bot:
[mediawiki/extensions/GlobalBlocking@master] Use IPUtils instead of deprecated IP class

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

CVE here. Thanks, @Reedy, @DannyS712, @Jdforrester-WMF for getting the backports done!

In T229731#5968376, @sbassett wrote:

CVE here. Thanks, @Reedy, @DannyS712, @Jdforrester-WMF for getting the backports done!

Not all the backports are merged yet - should this still be open?

Change 579428 merged by jenkins-bot:
[mediawiki/extensions/GlobalBlocking@REL1_33] SECURITY: Apply most specific global block and make sure applied block matches block showed on Special:Contributions

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

In T229731#5968480, @sbassett wrote:

In T229731#5968478, @DannyS712 wrote:

Not all the backports are merged yet - should this still be open?

I just +2'd 1.33, so that should merge soon. Looking at the issue for 1.31, it's complaining about an actual phpcs file, nothing related to GlobalBlocking, from what I can tell. In that case, we can likely just manually submit and file a bug regarding the phpcs issue in quibble, yes?

It's basically an instance of T233759: Update mediawiki/mediawiki-codesniffer for php7.3 support in release branches (or rather, would be fixed by it)

Change 579429 merged by Reedy:
[mediawiki/extensions/GlobalBlocking@REL1_31] SECURITY: Apply most specific global block and make sure applied block matches block showed on Special:Contributions

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