Repository: Spomky-Labs/otphp (php)

Issue:
Consider switching to paragonie/constant_time_encoding
| Q                    | A
| -------------------- | -----
| Bug report?          | no
| Feature request?     | yes
| BC Break report?     | no
| RFC? / Specification | no
| Library version      | 8.3.x, 9.x

The base 32 encoding library used by this project doesn't claim to protect against timing attacks.

It uses substr and strlen, which as per http://blog.ircmaxell.com/2014/11/its-all-about-time.html are not timing safe.

https://github.com/paragonie/constant_time_encoding is designed to prevent timing attacks

If you're interested in this change, could poke at it some.



Discussion:
Hi @larowlan,

This is a very interesting report. I will change both v8 and v9 branches in favor of this dependency.
As it is easier to bruteforce a n digits code, I am not sure there is a potential security issue at the moment, but that is a risk that I do not want to take.

Thanks.
Thanks, let me know if you want help, happy to work on it if you're in support of the change

Patch:
diff --git a/composer.json b/composer.json
--- a/composer.json
+++ b/composer.json
@@ -17,7 +17,7 @@
     ],
     "require": {
         "php": "^7.1",
-        "christian-riesen/base32": "^1.1",
+        "paragonie/constant_time_encoding": "^2.0",
         "beberlei/assert": "^2.4"
     },
     "require-dev": {
diff --git a/src/OTP.php b/src/OTP.php
--- a/src/OTP.php
+++ b/src/OTP.php
@@ -14,7 +14,7 @@
 namespace OTPHP;
 
 use Assert\Assertion;
-use Base32\Base32;
+use ParagonIE\ConstantTime\Base32;
 
 abstract class OTP implements OTPInterface
 {
@@ -108,7 +108,7 @@ protected function generateURI(string $type, array $options): string
      */
     private function getDecodedSecret(): string
     {
-        $secret = Base32::decode($this->getSecret());
+        $secret = Base32::decodeUpper($this->getSecret());
 
         return $secret;
     }
diff --git a/src/ParameterTrait.php b/src/ParameterTrait.php
--- a/src/ParameterTrait.php
+++ b/src/ParameterTrait.php
@@ -14,7 +14,7 @@
 namespace OTPHP;
 
 use Assert\Assertion;
-use Base32\Base32;
+use ParagonIE\ConstantTime\Base32;
 
 trait ParameterTrait
 {
@@ -205,7 +205,7 @@ protected function getParameterMap(): array
             },
             'secret'    => function ($value) {
                 if (null === $value) {
-                    $value = trim(Base32::encode(random_bytes(64)), '=');
+                    $value = trim(Base32::encodeUpper(random_bytes(64)), '=');
                 }
 
                 return $value;
diff --git a/src/TOTP.php b/src/TOTP.php
--- a/src/TOTP.php
+++ b/src/TOTP.php
@@ -147,7 +147,7 @@ public function getProvisioningUri(): string
      */
     private function timecode(int $timestamp): int
     {
-        return (int) ((($timestamp * 1000) / ($this->getPeriod() * 1000)));
+        return (int) floor($timestamp / $this->getPeriod());
     }
 
     /**


Test patch:
diff --git a/tests/TOTPTest.php b/tests/TOTPTest.php
--- a/tests/TOTPTest.php
+++ b/tests/TOTPTest.php
@@ -13,8 +13,8 @@
 
 namespace OTPHP\Test;
 
-use Base32\Base32;
 use OTPHP\TOTP;
+use ParagonIE\ConstantTime\Base32;
 use PHPUnit\Framework\TestCase;
 
 final class TOTPTest extends TestCase
@@ -137,9 +137,9 @@ public function testVectors($totp, $timestamp, $expected_value)
      */
     public function dataVectors()
     {
-        $totp_sha1 = $this->createTOTP(8, 'sha1', 30, Base32::encode('12345678901234567890'));
-        $totp_sha256 = $this->createTOTP(8, 'sha256', 30, Base32::encode('12345678901234567890123456789012'));
-        $totp_sha512 = $this->createTOTP(8, 'sha512', 30, Base32::encode('1234567890123456789012345678901234567890123456789012345678901234'));
+        $totp_sha1 = $this->createTOTP(8, 'sha1', 30, Base32::encodeUpper('12345678901234567890'));
+        $totp_sha256 = $this->createTOTP(8, 'sha256', 30, Base32::encodeUpper('12345678901234567890123456789012'));
+        $totp_sha512 = $this->createTOTP(8, 'sha512', 30, Base32::encodeUpper('1234567890123456789012345678901234567890123456789012345678901234'));
 
         return [
             [$totp_sha1,   59, '94287082'],