From c18ec155aeddb0d5383597fe7f1426b8566161da Mon Sep 17 00:00:00 2001 From: RobThree Date: Sat, 9 Jun 2018 12:09:24 +0200 Subject: [PATCH] * Minor refactoring of verifyCode. --- lib/TwoFactorAuth.php | 9 +++++++-- tests/TwoFactorAuthTest.php | 4 ++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/TwoFactorAuth.php b/lib/TwoFactorAuth.php index 5c50bd9..3e61503 100644 --- a/lib/TwoFactorAuth.php +++ b/lib/TwoFactorAuth.php @@ -83,11 +83,16 @@ class TwoFactorAuth { $timetamp = $this->getTime($time); - // To keep safe from timing-attachs we iterate *all* possible codes even though we already may have verified a code is correct + $timeslice = 0; + + // To keep safe from timing-attacks we iterate *all* possible codes even though we already may have + // verified a code is correct. We use the timeslice variable to hold either 0 (no match) or the timeslice + // of the match. Each iteration we either set the timeslice variable to the timeslice of the match + // or set the value to itself. This is an effort to maintain constant execution time for the code. for ($i = -$discrepancy; $i <= $discrepancy; $i++) { $ts = $timetamp + ($i * $this->period); $slice = $this->getTimeSlice($ts); - $timeslice += $this->codeEquals($this->getCode($secret, $ts), $code) ? $slice : 0; + $timeslice = $this->codeEquals($this->getCode($secret, $ts), $code) ? $slice : $timeslice; } return $timeslice > 0; diff --git a/tests/TwoFactorAuthTest.php b/tests/TwoFactorAuthTest.php index 979963e..accfd5d 100644 --- a/tests/TwoFactorAuthTest.php +++ b/tests/TwoFactorAuthTest.php @@ -170,6 +170,10 @@ class TwoFactorAuthTest extends PHPUnit_Framework_TestCase $this->assertEquals(47561575, $timeslice6); $this->assertEquals(true, $tfa->verifyCode('VMR466AB62ZBOKHE', '170645', 3, 1426847190, $timeslice7)); $this->assertEquals(47561576, $timeslice7); + + // Incorrect code should return false and a 0 timeslice + $this->assertEquals(false, $tfa->verifyCode('VMR466AB62ZBOKHE', '111111', 3, 1426847190, $timeslice8)); + $this->assertEquals(0, $timeslice8); } public function testTotpUriIsCorrect() {