PHPGangsta / GoogleAuthenticator

PHP class to generate and verify Google Authenticator 2-factor authentication
http://phpgangsta.de/4376
BSD 2-Clause "Simplified" License
2.26k stars 698 forks source link

Fix timing attack vulnerability #18

Closed yohgaki closed 8 years ago

yohgaki commented 9 years ago

This comparison is not timing safe $calculatedCode == $code

Therefore, it's vulnerable to timing attack. This PR fixes it.

jippi commented 9 years ago

:+1

Conver commented 8 years ago

+1

yohgaki commented 8 years ago

@egarcam Comparing string length does not fix timing attack vulnerability because timing difference happens with C string comparison functions like strncmp() or like. i.e. These functions terminate comparison as soon as they detect difference, so there is a little execution difference and attackers can detect 2FA token one by one by statistical analysis.

Since token character is limited to [0-9] and 6 digits, it is relatively easy to attack by timing. Risk is not too high, but it's better to be fixed as this is about authentication.

NOTE: PHP does not provide timing safe string comparison for == nor ===. Users may use hash_equals() for timing safe string comparison which is available from PHP 5.6. I didn't use it because I can avoid string comparison by int comparison in this case. This works with PHP 5.5. or less.