From TOTPKey.php:
foreach ( $this->scratchTokens as $i => $scratchToken ) {
if ( $token === $scratchToken ) {Should use hash_equals() instead. This is unlikely to be practically exploitable because of the rate limiting.
There's also if ( $window > $lastWindow && $result->toHOTP( 6 ) === $token ) {, but I would think that's fine and doesn't need the constant time comparison? It probably doesn't hurt to be safe I guess.
Patch that fixes both of these:
Code-Review +2
We've recently pushed some of these through gerrit as "code hardening", which I think is probably still ok to do.
Ack, sounds good to me. Because of the loops and that we're checking equality against multiple values, it seems pretty hard to pull off an exploit.
Change 765341 had a related patch set uploaded (by SBassett; author: SBassett):
[mediawiki/extensions/OATHAuth@master] SECURITY: Use constant time checks for token values
Change 765366 had a related patch set uploaded (by Reedy; author: SBassett):
[mediawiki/extensions/OATHAuth@REL1_37] SECURITY: Use constant time checks for token values
Change 765367 had a related patch set uploaded (by Reedy; author: SBassett):
[mediawiki/extensions/OATHAuth@REL1_36] SECURITY: Use constant time checks for token values
Change 765368 had a related patch set uploaded (by Reedy; author: SBassett):
[mediawiki/extensions/OATHAuth@REL1_35] SECURITY: Use constant time checks for token values
Change 765341 merged by jenkins-bot:
[mediawiki/extensions/OATHAuth@master] SECURITY: Use constant time checks for token values
Change 765366 merged by jenkins-bot:
[mediawiki/extensions/OATHAuth@REL1_37] SECURITY: Use constant time checks for token values
Change 765367 merged by jenkins-bot:
[mediawiki/extensions/OATHAuth@REL1_36] SECURITY: Use constant time checks for token values
Change 765368 merged by jenkins-bot:
[mediawiki/extensions/OATHAuth@REL1_35] SECURITY: Use constant time checks for token values