CentralAuthUser::validateAuthToken should use constant-time string comparison

Closed, ResolvedPublic

Actions

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

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

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

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