Page MenuHomePhabricator

CentralAuth doesn't check if given valid but unusable name
Closed, ResolvedPublic

Description

CentralAuth doesn't check if the user name it was given is usable, only if it's valid.

For MediaWiki with SessionManager, this isn't too bad of a problem: MediaWiki\Session\UserInfo::newFromName does throw an exception if given an unusable name, which makes the providers throw rather than failing more gracefully. This is why {T130099} was giving a 503 error rather than failing in a more user-friendly manner.

For MediaWiki without SessionManager, though, it's a security issue: It apparently just allows the login, which is why Xqbot only recently started failing as described in T130099, instead of failing as soon as Xqt mistranslated "Redirect fixer" on translatewiki back in 2009.

Since we'll probably want to backport a fix to 1.26 and earlier, I'm going to file this as a security bug.

Event Timeline

Anomie created this task.Mar 18 2016, 5:04 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 18 2016, 5:04 PM
Anomie claimed this task.Mar 18 2016, 5:19 PM
Anomie added a project: Patch-For-Review.

Patches:

  • For master/1.27:
  • For backports:
Bawolff triaged this task as Normal priority.Mar 22 2016, 9:38 PM
Bawolff added a subscriber: Bawolff.

I'm prioritizing this as normal, since someone impersonating an internal mediawiki user is on the low end of evil things to do.

This seems like it contributed to T139970#2465971, apparently Echo's junk triggered on a system user and flooded the API with requests which blew up as described in the description instead of being handled more gracefully.

dpatrick added a subscriber: dpatrick.

Patches:

  • For master/1.27:
  • For backports:

Thanks for backporting. We'll test this this week, and deploy on July 25.

dpatrick added a comment.EditedAug 8 2016, 9:31 PM

@Anomie b1908c5ada1e219488b12bc2e0d1ac53d19315b0 removed includes/session/CentralAuthSessionCompat.php. Can you update this patch to account for that?

In other words, I'm asking you, @Anomie, to verify that the patches can be used as they are, sans the includes/session/CentralAuthSessionCompat.php portions.

NM. Looks sane to me and tests fine. Here's the updated version.

dpatrick closed this task as Resolved.Aug 8 2016, 9:58 PM

21:57 dapatrick: Deployed patch for T130384 to wmf.13

Yeah, no need to patch deleted code. I'm glad you didn't need to wait for me to get back from vacation (:

Anomie changed the visibility from "Custom Policy" to "Public (No Login Required)".Aug 15 2016, 5:13 PM
Anomie changed Security from Software security bug to None.
Restricted Application added a subscriber: Malyacko. · View Herald TranscriptAug 15 2016, 5:13 PM

Change 304861 merged by Jforrester:
SECURITY: Check for valid but unusable user names

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