Page MenuHomePhabricator

Unable to set up move protection in ns:0 on Commons
Closed, ResolvedPublicSecurity

Description

Problem:
Wikibase takes ownership of the namespace reserved for Items (as well as those for other Entity types like Properties, Lexemes) - the main namespace on Wikidata. It prevents moving of pages there. This behavior of taking over the main namespace is not correct on Commons. It leads to the main namespace on Commons not being move-protectable. That's bad!

We additionally need to check:

  • Is the main namespace hardcoded here? Wikibase allows Items to be in a different namespace and the move protection imposed by Wikibase should trigger there, not always the main namespace.

BDD
GIVEN
AND
WHEN
AND
THEN
AND

Acceptance criteria:

  • Wikibase only triggers its own move protection in the Entity namespaces it is responsible for.

Original report:
Today, https://commons.wikimedia.org/wiki/Hauptseite was (accidentally?) moved by a non-sysop. The most recent entry in the page protection log includes a move protection for non-sysops ([edit=autoconfirmed] (indefinite) ‎[move=sysop] (indefinite)), so actually, this action should not have been possible. However, the page information does only show the edit protection and no move protection. When trying to protect the page, there is no option to set up a move protection, the only option is edit protection. The same issue applies to other pages in the gallery namespace and is not limited to this page.

Event Timeline

Didym created this task.Jul 18 2020, 2:02 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 18 2020, 2:02 PM
Urbanecm set Security to Software security bug.Jul 18 2020, 5:18 PM
Urbanecm added projects: Security, Security-Team.
Urbanecm changed the visibility from "Public (No Login Required)" to "Custom Policy".
Urbanecm changed the subtype of this task from "Task" to "Security Issue".
Urbanecm added a subscriber: Urbanecm.

potentional security vulnerability

Urbanecm triaged this task as High priority.Jul 18 2020, 5:24 PM

Confirmed at commons beta, cannot confirm at cswiki beta.

So...here is the story:

Title::getRestrictionTypes() calls TitleGetRestrictionTypes hook, which is received by Wikibase\Repo\RepoHooks::onTitleGetRestrictionTypes, which has the following codepart:

if ( $namespaceLookup->isEntityNamespace( $title->getNamespace() ) ) {
	// Remove create and move protection for Wikibase namespaces
	$types = array_diff( $types, [ 'create', 'move' ] );
}

It seems it wrongfully considers Commons's main namespace as namespace for instances, which doesn't have move protection allowed. Unlike Wikidata, Commons's main namespace isn't reserved for Q items and can be actually move protected.

Tagging relevant projects to have a look.

Urbanecm added a subscriber: Ladsgroup.

@Ladsgroup Could you please help to triage this from the Wikibase side? Thanks!

Hey, Adding PMs of SDC and Wikidata. This seems severe.

adding Tonina as the coming week's incident manager and Adam to have it on his radar. We need to get this fixed.

sbassett moved this task from Incoming to Watching on the Security-Team board.Jul 20 2020, 3:33 PM
toan added a comment.EditedJul 21 2020, 12:58 PM

Wikimedia Commons & Wikidata example

The following example shows Wikimedia Commons using entities from Wikidata.org:

$entitySources = [
    'wikidata' => [
        'entityNamespaces' => [
            'item' => 0,
            'property' => 120,
            'lexeme' => 146,
        ],
        'repoDatabase' => 'wikidatawiki',
        'baseUri' => 'http://www.wikidata.org/entity/',
        'rdfNodeNamespacePrefix' => 'wd',
        'rdfPredicateNamespacePrefix' => '',
        'interwikiPrefix' => 'd',
    ],
    'commons' => [
        'entityNamespaces' => [
            'mediainfo' => '6/mediainfo',
        ],
        'repoDatabase' => 'commonswiki',
        'baseUri' => 'https://commons.wikimedia.org/wiki/Special:EntityData/',
        'rdfNodeNamespacePrefix' => 'sdc',
        'rdfPredicateNamespacePrefix' => 'sdc',
        'interwikiPrefix' => 'c',
    ],
];
$wgWBRepoSettings['entitySources'] = $entitySources;
$wgWBClientSettings['entitySources'] = $entitySources;

Further things to check/investigate/understand:

  • is it possible to protect a page in any non-entity namespace in a repo against moving? E.g. can one actually protect https://www.wikidata.org/wiki/Wikidata:Sandbox against moving?
  • is it possible to protect a page in the main namespace in a local repo/wiki where items are not in the main namespace against moving?

@Michael

is it possible to protect a page in any non-entity namespace in a repo against moving? E.g. can one actually protect https://www.wikidata.org/wiki/Wikidata:Sandbox against moving?

Tried that at https://test.wikidata.org/wiki/Wikidata:Sandbox, managed to successfully do so (verified via my WMF account, which has no advanced rights).

is it possible to protect a page in the main namespace in a local repo/wiki where items are not in the main namespace against moving?

No, it isn't possible to move-protect pages in main namespace, even when items aren't in the main namespace. For me, the protection interface looks like this:

instead of something like this:

which is from my user sandbox page (in NS_USER).

@Michael To avoid confusion, the interface doesn't have any option for move-protection for pages in NS_MAIN at commonswiki - the main page appears to be move-protected only because it was already move-protected before this bug was introduced.

So...here is the story:

Title::getRestrictionTypes() calls TitleGetRestrictionTypes hook, which is received by Wikibase\Repo\RepoHooks::onTitleGetRestrictionTypes, which has the following codepart:

if ( $namespaceLookup->isEntityNamespace( $title->getNamespace() ) ) {
	// Remove create and move protection for Wikibase namespaces
	$types = array_diff( $types, [ 'create', 'move' ] );
}

It seems it wrongfully considers Commons's main namespace as namespace for instances, which doesn't have move protection allowed.

Just to confirm – yes, it indeed considers ns0 as an entity namespace:

lucaswerkmeister-wmde@mwdebug1002:~$ mwscript shell.php commonswiki
Psy Shell v0.10.4 (PHP 7.2.31-1+0~20200514.41+debian9~1.gbpe2a56b+wmf1 — cli) by Justin Hileman
>>> use \Wikibase\Repo\WikibaseRepo;
>>> $namespaceLookup = WikibaseRepo::getDefaultInstance()->getEntityNamespaceLookup();
=> Wikibase\Lib\Store\EntityNamespaceLookup {#527}
>>> $namespaceLookup->isEntityNamespace( 0 );
=> true

NS_FILE, on the other hand, is not considered an entity namespace:

>>> $namespaceLookup->isEntityNamespace( NS_FILE )
=> false

But ns120 (Property on Wikidata) is considered an entity namespace:

>>> $namespaceLookup->isEntityNamespace( 120 )
=> true

On Commons, that namespace doesn’t even exist.

/me impersonates gerritbot

Change 616109 had a related patch set uploaded (by Itamar Givon; owner: Itamar Givon):
[mediawiki/extensions/Wikibase@master] Prevent onTitleGetRestrictionTypes changing ns0 protections

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


That change should fix the issue (and I think it’s okay that it’s public, it doesn’t make this issue obvious). Is this serious enough to justify a Friday deployment, or should we wait until Monday to backport it?

Change 616032 had a related patch set uploaded (by Itamar Givon; owner: Lucas Werkmeister):
[mediawiki/extensions/Wikibase@wmf/1.36.0-wmf.1] Prevent onTitleGetRestrictionTypes changing ns0 protections

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


Seems serious enough for a Friday deployment, Amir will deploy that backport once CI is done.

greg added a subscriber: greg.Jul 24 2020, 4:53 PM
09:47:41 	<+logmsgbot>	!log ladsgroup@deploy1001 Synchronized php-1.36.0-wmf.1/extensions/Wikibase/repo/includes/WikibaseRepo.php: [[gerrit:616032|Prevent onTitleGetRestrictionTypes changing ns0 protections]], Part I (duration: 01m 06s)
09:47:45 	<+stashbot>	Logged the message at https://wikitech.wikimedia.org/wiki/Server_Admin_Log
09:49:30 	<+logmsgbot>	!log ladsgroup@deploy1001 Synchronized php-1.36.0-wmf.1/extensions/Wikibase/repo/includes/RepoHooks.php: [[gerrit:616032|Prevent onTitleGetRestrictionTypes changing ns0 protections]], Part II (duration: 01m 07s)
09:49:33 	<+stashbot>	Logged the message at https://wikitech.wikimedia.org/wiki/Server_Admin_Log

We got this deployed and it seems it fixes the issue, I monitor logs for any errors in the next couple of days.

/me impersonates stashbot (edit: sorry, hadn’t seen greg’s comment yet)

Mentioned in SAL (#wikimedia-cloud) [2020-07-24T16:47Z] <ladsgroup@deploy1001> Synchronized php-1.36.0-wmf.1/extensions/Wikibase/repo/includes/WikibaseRepo.php: [[gerrit:616032|Prevent onTitleGetRestrictionTypes changing ns0 protections]], Part I (duration: 01m 06s)

Mentioned in SAL (#wikimedia-cloud) [2020-07-24T16:49Z] <ladsgroup@deploy1001> Synchronized php-1.36.0-wmf.1/extensions/Wikibase/repo/includes/RepoHooks.php: [[gerrit:616032|Prevent onTitleGetRestrictionTypes changing ns0 protections]], Part II (duration: 01m 07s)

Seems to be fixed on Commons now (trying to move Hauptseite shows an error again).

Horray, I can move-protect commons pages too!

I'm also able to protect non-existent pages against creation:

I'm also not able to do the same at Wikidata.

Thanks everyone!

Urbanecm added a subscriber: sbassett.

I think this can go public now. @sbassett @Ladsgroup @Addshore Opinions on that?

Once done, this should go to User-notice I think.

I think this can go public now. @sbassett @Ladsgroup @Addshore Opinions on that?

Yep, everything important already went through gerrit. And I'm not seeing anything sensitive on this task that still requires protection.

Once done, this should go to User-notice I think.

Probably a good idea. At least some sort of public issue summary (besides this task) is likely warranted.

sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".Jul 24 2020, 7:00 PM
sbassett removed a project: Patch-For-Review.
sbassett moved this task from Watching to Our Part Is Done on the Security-Team board.
RhinosF1 added a subscriber: RhinosF1.EditedJul 24 2020, 7:02 PM

There was a restricted task merged into this. Should it be made public as well? (https://phabricator.wikimedia.org/T258323#6317139)

There was a restricted task merged into this. Should it be made public as well? (https://phabricator.wikimedia.org/T258323#6317139)

Done.

There was a restricted task merged into this. Should it be made public as well? (https://phabricator.wikimedia.org/T258323#6317139)

Done.

Thanks!

Base added a subscriber: Base.Jul 26 2020, 10:48 AM
Jony added a subscriber: Jony.Jul 29 2020, 1:47 PM

Is this intended to be summarized in TechNews this week? If so, could someone please write a short (1-2 sentence) clear summary for inclusion? Thanks.

Suggested summary:

A bug in the Wikibase extension inadvertently disabled the “move” and “create” protection types in the main (Gallery) namespace on Wikimedia Commons. New protections could not be added, and existing protections were not enforced, allowing some page moves and creations that should not have been possible.

Lydia_Pintscher closed this task as Resolved.Aug 11 2020, 9:50 AM
Lydia_Pintscher claimed this task.

Thank you everyone!