CentralAuthUser::validateAuthToken should use constant-time string comparison
Closed, ResolvedPublic
Actions

Details

Related Gerrit Patches:

mediawiki/extensions/CentralAuth : master

Tgr raised the priority of this task from to Needs Triage.

Tgr updated the task description. (Show Details)

Tgr changed the visibility from "Public (No Login Required)" to "Custom Policy".

Tgr changed the edit policy from "All Users" to "Custom Policy".

Tgr changed Security from None to Software security bug.

Tgr added a subscriber: Tgr.

• dpatrick triaged this task as Low priority.

Use constant time comparison in CentralAuthUser::validateAuthToken828 BDownload

Given its non-trivial to pull off a timing attack of this type, I wonder if this patch could go directly on gerrit (/me looks at @csteipp )

In T125290#2027702, @Bawolff wrote:

Use constant time comparison in CentralAuthUser::validateAuthToken828 BDownload

Minor, but the argument order should be reversed. From http://php.net/manual/en/function.hash-equals.php.

Note:
It is important to provide the user-supplied string as the second parameter, rather than the first.

eww, talk about an easy to misuse api.

I'll update the patch in a moment

do auth token comparision in constant time828 BDownload

In T125290#2033537, @Bawolff wrote:

eww, talk about an easy to misuse api.

Google suggests its for future compatability reasons (https://stackoverflow.com/questions/27911597/why-is-order-of-arguments-in-phps-hash-equals-function-important). I guess that's more reasonable. I still think that's a poor api design.

Deployed

23:49 csteipp: deployed patch for T125290

csteipp closed this task as Resolved.Feb 19 2016, 12:00 AM

demon changed the visibility from "Custom Policy" to "Public (No Login Required)".

demon changed the edit policy from "Custom Policy" to "All Users".

demon changed Security from Software security bug to None.

Change 284237 merged by Chad:
[SECURITY] Use constant time comparison in validateAuthToken