Page MenuHomePhabricator

Using auto promote APCOND_BLOCKED can cause php to OOM
Closed, ResolvedPublicSecurity

Description

Today I discovered that using 8 & 9 parameters for AutoPromote (["!",8,9]), causes php to oom. I only discovered this after investigating one of our wikis OOM'ing.

I reproduced by doing:

MariaDB [mhglobal]> update mw_permissions set perm_autopromote = '["!",8,9]' where perm_dbname = 'pkvastwiki' and perm_group = 'user';

(our automation to set wgAutopromote).

Debugging a bit more I see getBlock causes an oom here https://github.com/wikimedia/mediawiki/blob/04fb8689b4d90e1c4cc7fe5aef2999492c21ebe4/includes/user/User.php#L1894

Digging deeper I get to https://github.com/wikimedia/mediawiki/blob/REL1_35/includes/block/BlockManager.php#L132

then digging even deeper and I get to https://github.com/wikimedia/mediawiki/blob/REL1_35/includes/Permissions/PermissionManager.php#L1293

Either there is a new request or this isn't caching as logging it, I get a load of empties thus that if statement keeps executing.

Event Timeline

@Reedy suggested that there might be a infinite loop.

2020-12-15 01:08:23 test2 test2wiki: [33f96910118a256c243e33e8] /w/api.php?action=query&format=json&titles=Main_Page&prop=categories&clprop=sortkey   ErrorException from line 535 of /srv/mediawiki/w/includes/user/UserGroupManager.php: PHP Warning: error_log() expects parameter 1 to be string, array given
#0 [internal function]: MWExceptionHandler::handleError(integer, string, string, integer, array)
#1 /srv/mediawiki/w/includes/user/UserGroupManager.php(535): error_log(array, integer)
#2 /srv/mediawiki/w/includes/user/UserGroupManager.php(484): MediaWiki\User\UserGroupManager->checkCondition(array, User)
#3 /srv/mediawiki/w/includes/user/UserGroupManager.php(470): MediaWiki\User\UserGroupManager->recCheckCondition(array, User)
#4 /srv/mediawiki/w/includes/user/UserGroupManager.php(370): MediaWiki\User\UserGroupManager->recCheckCondition(array, User)
#5 /srv/mediawiki/w/includes/user/UserGroupManager.php(257): MediaWiki\User\UserGroupManager->getUserAutopromoteGroups(User)
#6 /srv/mediawiki/w/includes/user/UserGroupManager.php(295): MediaWiki\User\UserGroupManager->getUserImplicitGroups(User, integer, boolean)
#7 /srv/mediawiki/w/includes/user/User.php(2967): MediaWiki\User\UserGroupManager->getUserEffectiveGroups(User, integer, boolean)
#8 /srv/mediawiki/w/includes/Permissions/PermissionManager.php(1295): User->getEffectiveGroups()
#9 /srv/mediawiki/w/includes/Permissions/PermissionManager.php(1244): MediaWiki\Permissions\PermissionManager->getUserPermissions(User)
#10 /srv/mediawiki/w/includes/block/BlockManager.php(132): MediaWiki\Permissions\PermissionManager->userHasRight(User, string)
#11 /srv/mediawiki/w/includes/user/User.php(1603): MediaWiki\Block\BlockManager->getUserBlock(User, WebRequest, boolean)
#12 /srv/mediawiki/w/includes/user/User.php(1986): User->getBlockedStatus(boolean)
#13 /srv/mediawiki/w/includes/block/BlockManager.php(472): User->getBlock()
#14 /srv/mediawiki/w/includes/MediaWiki.php(767): MediaWiki\Block\BlockManager->trackBlockWithCookie(User, WebResponse)
#15 /srv/mediawiki/w/includes/api/ApiMain.php(555): MediaWiki::preOutputCommit(DerivativeContext)
#16 /srv/mediawiki/w/includes/api/ApiMain.php(500): ApiMain->executeActionWithErrorHandling()
#17 /srv/mediawiki/w/api.php(90): ApiMain->execute()
#18 /srv/mediawiki/w/api.php(45): wfApiMain()

is a traceback from when it works.

Could the infinite loop be checkCondition calls getBlock() and getBlock eventually ends back up at checkCondition and since and repeat?

Issue seems to only occur if using "8" and not "8" and "9".

To reproduce with eval.php do the following:

Set https://github.com/wikimedia/mediawiki/blob/master/includes/user/User.php#L1601 to true (since a request isn't set via eval thus will be null)

run the following

$u = User::newFromName( $name ); $u->getBlock();

you should see it just continuously executing.

Also here's a well wrote description:

https://github.com/wikimedia/mediawiki/blob/REL1_35/includes/user/UserGroupManager.php#L534 fatals. ->getBlock is called, which calls getBlockedStatus, which called getUserBlock (https://github.com/wikimedia/mediawiki/blob/REL1_35/includes/user/User.php#L1602) which then calls userHasRights (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/block/BlockManager.php#L131), which then calls getUserPermissions (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/Permissions/PermissionManager.php#L1273), which then calls getEffectiveGroups (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/Permissions/PermissionManager.php#L1325), which then calls getUserEffectiveGroups (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/user/User.php#L2876), which then calls getImplicitGroups (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/user/UserGroupManager.php#L298), which then calls getAutopromoteGroups (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/user/UserGroupManager.php#L260), which then calls recCheckConditions (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/user/UserGroupManager.php#L372), which then calls checkCondition (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/user/UserGroupManager.php#L486), which if contains a isBlocked conditions, called getBlock, which then called getBlockedStatus... and so on

Our config:

["!",8,9]

Also the user in question isn't blocked (me).

> var_dump($wgAutopromote);
array(6) {
  ["autoconfirmed"]=>
  array(3) {
    [0]=>
    string(1) "&"
    [1]=>
    array(2) {
      [0]=>
      int(1)
      [1]=>
      int(10)
    }
    [2]=>
    array(2) {
      [0]=>
      int(2)
      [1]=>
      int(345600)
    }
  }
  ["user"]=>
  array(3) {
    [0]=>
    string(1) "!"
    [1]=>
    int(8)
    [2]=>
    int(9)
  }
  ["checkuser"]=>
  array(0) {
  }
  ["interwiki-admin"]=>
  array(0) {
  }
  ["oversight"]=>
  array(0) {
  }
  ["steward"]=>
  array(0) {
  }
}
Paladox triaged this task as High priority.Dec 15 2020, 4:32 PM

Patch below

commit 87074ef97a04a30fa905c1d241ebb4068ab0c506
Author: paladox <thomasmulhall410@yahoo.com>
Date:   Tue Dec 15 20:38:17 2020 +0000

    Fix infinite recursion with using getBlock within checkCondition
    
    The problem was that when using $user->getBlock
    within checkCondition (APCOND_BLOCKED) caused a infinite
    recursion as calling getBlock eventually leads back to
    checkCondition.
    
    The fix here is to stop using userHasRight as that is the cause of the recursion.
    
    Bug: T270145
    Change-Id: I9b5eb94f9706fd1ea4e8c979302b163683746fd6

diff --git a/includes/block/BlockManager.php b/includes/block/BlockManager.php
index 38010b55ce..4f3ef6701c 100644
--- a/includes/block/BlockManager.php
+++ b/includes/block/BlockManager.php
@@ -23,6 +23,7 @@ namespace MediaWiki\Block;
 use DateTime;
 use DateTimeZone;
 use LogicException;
+use MediaWiki\MediaWikiServices;
 use MediaWiki\Config\ServiceOptions;
 use MediaWiki\HookContainer\HookContainer;
 use MediaWiki\HookContainer\HookRunner;
@@ -128,7 +129,7 @@ class BlockManager {
 		// or they may be exempt (case #2). If affected, look for additional blocks
 		// against the IP address and referenced in a cookie.
 		$checkIpBlocks = $request &&
-			!$this->permissionManager->userHasRight( $user, 'ipblock-exempt' );
+			!$this->getUserPermission( 'ipblock-exempt', $user );
 
 		if ( $request && $checkIpBlocks ) {
 
@@ -170,6 +171,20 @@ class BlockManager {
 		return $block;
 	}
 
+	/**
+	 * Gets specified permission (T270145)
+	 *
+	 * @param string $perm
+	 * @param UserIdentity $user
+	 * @return bool
+	 */
+	private function getUserPermission( string $perm, UserIdentity $user ) {
+		$userGroups = MediaWikiServices::getInstance()->getUserGroupManager();
+		return in_array( $perm, MediaWikiServices::getInstance()
+			->getPermissionManager()
+			->getGroupPermissions( $userGroups->getUserGroups( $user ) ) );
+	}
+
 	/**
 	 * Get the cookie block, if there is one.
 	 *

To reproduce with eval.php do the following:

Set https://github.com/wikimedia/mediawiki/blob/master/includes/user/User.php#L1601 to true (since a request isn't set via eval thus will be null)

run the following

$u = User::newFromName( $name ); $u->getBlock();

you should see it just continuously executing.

Vanilla master MW works fine (with or without the patch to User), with the default $wgAutopromote config:

> var_dump( $wgAutopromote );
/var/www/wiki/mediawiki/core/maintenance/eval.php(82) : eval()'d code:1:
array(1) {
  'autoconfirmed' =>
  array(3) {
    [0] =>
    string(1) "&"
    [1] =>
    array(2) {
      [0] =>
      int(1)
      [1] =>
      int(0)
    }
    [2] =>
    array(2) {
      [0] =>
      int(2)
      [1] =>
      int(0)
    }
  }
}
> echo time() . "\n"; $u = User::newFromName( 'Reedy' ); $u->getBlock(); echo time() . "\n";
1608070451
1608070451

Rebuilding the var_dump, I think your $wgAutopromote is

$wgAutopromote = [
  "autoconfirmed" => [ "&", [ 1, 1 ], [ 2, 345600] ],
  "user" => [ "!", 8, 9],
  "checkuser" => [],
  "interwiki-admin" => [],
  "oversight" => [],
  "steward" => [],
];

And the more human readable version

$wgAutopromote = [
  "autoconfirmed" => [ "&", [ APCOND_EDITCOUNT, 1 ], [ APCOND_AGE, 345600] ],
  "user" => [ "!", APCOND_BLOCKED, APCOND_ISBOT ],
  "checkuser" => [],
  "interwiki-admin" => [],
  "oversight" => [],
  "steward" => [],
];

Again, on a vanilla MW core install (without the change to User.php), with your $wgAutopromote I don't get any sort of error:

> echo time() . "\n"; $u = User::newFromName( 'Reedy' ); $u->getBlock(); echo time() . "\n";
1608071987
1608071987

I see the same behaviour on 1.35 too (ie it's not broken) with either config.

However, you do mention an extension, which is not hosted here, and it's not linked to, so can't advise on the code. But it's also not mentioned in the apparent infinite recursion loop stack trace...

Now, on my 1.35 wiki (and similar on my master wiki), I'm already in some groups with privileges...

/var/www/wiki-1.35/core/maintenance/eval.php(78) : eval()'d code:1:
array(3) {
  [0] =>
  string(10) "bureaucrat"
  [1] =>
  string(15) "interface-admin"
  [2] =>
  string(5) "sysop"
}

Information not provided about that in this case.

Also here's a well wrote description:

https://github.com/wikimedia/mediawiki/blob/REL1_35/includes/user/UserGroupManager.php#L534 fatals. ->getBlock is called, which calls getBlockedStatus, which called getUserBlock (https://github.com/wikimedia/mediawiki/blob/REL1_35/includes/user/User.php#L1602) which then calls userHasRights (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/block/BlockManager.php#L131), which then calls getUserPermissions (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/Permissions/PermissionManager.php#L1273), which then calls getEffectiveGroups (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/Permissions/PermissionManager.php#L1325), which then calls getUserEffectiveGroups (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/user/User.php#L2876), which then calls getImplicitGroups (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/user/UserGroupManager.php#L298), which then calls getAutopromoteGroups (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/user/UserGroupManager.php#L260), which then calls recCheckConditions (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/user/UserGroupManager.php#L372), which then calls checkCondition (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/user/UserGroupManager.php#L486), which if contains a isBlocked conditions, called getBlock, which then called getBlockedStatus... and so on

It's not very readable... Let's add some newlines

https://github.com/wikimedia/mediawiki/blob/REL1_35/includes/user/UserGroupManager.php#L534 fatals.

->getBlock is called which calls getBlockedStatus
which called getUserBlock (https://github.com/wikimedia/mediawiki/blob/REL1_35/includes/user/User.php#L1602)
which then calls userHasRights (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/block/BlockManager.php#L131)
which then calls getUserPermissions (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/Permissions/PermissionManager.php#L1273)
which then calls getEffectiveGroups (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/Permissions/PermissionManager.php#L1325)
which then calls getUserEffectiveGroups (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/user/User.php#L2876)
which then calls getImplicitGroups (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/user/UserGroupManager.php#L298)
which then calls getAutopromoteGroups (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/user/UserGroupManager.php#L260)
which then calls recCheckConditions (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/user/UserGroupManager.php#L372)
which then calls checkCondition (https://github.com/wikimedia/mediawiki/blob/6c9154b6faaff0813612a2475fb1974f5f1aad5b/includes/user/UserGroupManager.php#L486)
which if contains a isBlocked conditions called getBlock which then called getBlockedStatus... and so on

Which helps a little, except it doesn't help me redproduce the issue.

There's seemingly some missing detail/config here that's probably the key to why it's failing as it is

And as I think I said on IRC before...

It should be possible to craft a test to exercise and show this error condition

Setup the autopromote config, set up the user in terms of groups/rights/blocks/whatever...

It should fail without your fix. So put it as the parent commit of the "fix", that should fail, and then the parent should pass CI...

Reedy renamed this task from Using AutoPromotes 8 (must be blocked) & 9 (bot) can cause php to OOM to Using auto promote APCOND_BLOCKED can cause php to OOM.Dec 17 2020, 10:28 PM
Reedy assigned this task to Paladox.
Reedy added a project: MW-1.35-release.
Reedy changed the visibility from "Custom Policy" to "Public (No Login Required)".
Reedy changed the edit policy from "Custom Policy" to "All Users".

Change 654865 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Revert "Fix infinite recursion with wgAutopromote when using getBlock within checkCondition"

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

Change 655148 had a related patch set uploaded (by Paladox; owner: Paladox):
[mediawiki/core@master] Fix fetching ipblock-exempt within BlockManager#getUserBlock

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

Change 658940 had a related patch set uploaded (by Urbanecm; owner: Paladox):
[mediawiki/core@wmf/1.36.0-wmf.27] Fix fetching ipblock-exempt within BlockManager::getUserBlock

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

Change 658941 had a related patch set uploaded (by Urbanecm; owner: Paladox):
[mediawiki/core@wmf/1.36.0-wmf.28] Fix fetching ipblock-exempt within BlockManager::getUserBlock

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

Change 654865 abandoned by Urbanecm:
[mediawiki/core@master] Revert "Fix infinite recursion with wgAutopromote when using getBlock within checkCondition"

Reason:
a real fix was 2'ed

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

Change 658942 had a related patch set uploaded (by Urbanecm; owner: Paladox):
[mediawiki/core@REL1_35] Fix fetching ipblock-exempt within BlockManager::getUserBlock

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

Change 658942 merged by jenkins-bot:
[mediawiki/core@REL1_35] Fix fetching ipblock-exempt within BlockManager::getUserBlock

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

Change 655148 merged by jenkins-bot:
[mediawiki/core@master] Fix fetching ipblock-exempt within BlockManager::getUserBlock

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

Change 658940 merged by jenkins-bot:
[mediawiki/core@wmf/1.36.0-wmf.27] Fix fetching ipblock-exempt within BlockManager::getUserBlock

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

Change 658941 merged by jenkins-bot:
[mediawiki/core@wmf/1.36.0-wmf.28] Fix fetching ipblock-exempt within BlockManager::getUserBlock

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

Mentioned in SAL (#wikimedia-operations) [2021-01-28T00:31:25Z] <urbanecm@deploy1001> Synchronized php-1.36.0-wmf.28/includes/: rMWa67fe4f7cbf1: Fix fetching ipblock-exempt within BlockManager::getUserBlock (T271551, T270145) (duration: 01m 07s)

Mentioned in SAL (#wikimedia-operations) [2021-01-28T00:33:10Z] <urbanecm@deploy1001> Synchronized php-1.36.0-wmf.27/includes/: rMWc5c39ba8b3fc: Fix fetching ipblock-exempt within BlockManager::getUserBlock (T271551, T270145) (duration: 01m 04s)