Page MenuHomePhabricator

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

Event Timeline

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.
Bawolff added a subscriber: csteipp.

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 )

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

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

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

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