From 26086981a24e72f283da38dbdb992f27b4135a80 Mon Sep 17 00:00:00 2001
From: Aleksander Machniak <alec@alec.pl>
Date: Tue, 08 Sep 2015 11:38:19 -0400
Subject: [PATCH] Improve randomness of security tokens (#1490529)

---
 CHANGELOG                             |    1 
 program/include/rcmail.php            |    2 
 tests/Framework/Utils.php             |    2 
 .htaccess                             |    2 
 program/lib/Roundcube/rcube_utils.php |   62 ++++++++++++++++++++++++++++---
 5 files changed, 60 insertions(+), 9 deletions(-)

diff --git a/.htaccess b/.htaccess
index a2b516c..089e014 100644
--- a/.htaccess
+++ b/.htaccess
@@ -32,7 +32,7 @@
 # security rules:
 # - deny access to files not containing a dot or starting with a dot
 #   in all locations except installer directory
-RewriteRule ^(?!installer|[a-f0-9]{16})(\.?[^\.]+)$ - [F]
+RewriteRule ^(?!installer|[a-zA-Z0-9]{16})(\.?[^\.]+)$ - [F]
 # - deny access to some locations
 RewriteRule ^/?(\.git|\.tx|SQL|bin|config|logs|temp|tests|program\/(include|lib|localization|steps)) - [F]
 # - deny access to some documentation files
diff --git a/CHANGELOG b/CHANGELOG
index 6472e26..f28a95d 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
 CHANGELOG Roundcube Webmail
 ===========================
 
+- Security: Improve randomness of security tokens (#1490529)
 - Security: Use random security tokens instead of hashes based on encryption key (#1490404)
 - Security: Improved encrypt/decrypt methods with option to choose the cipher_method (#1489719)
 - Make optional adding of standard signature separator - sig_separator (#1487768)
diff --git a/program/include/rcmail.php b/program/include/rcmail.php
index a7c5a91..b2ab79a 100644
--- a/program/include/rcmail.php
+++ b/program/include/rcmail.php
@@ -810,7 +810,7 @@
 
             // remove old token from the path
             $base_path = rtrim($base_path, '/');
-            $base_path = preg_replace('/\/[a-f0-9]{' . strlen($token) . '}$/', '', $base_path);
+            $base_path = preg_replace('/\/[a-zA-Z0-9]{' . strlen($token) . '}$/', '', $base_path);
 
             // this need to be full url to make redirects work
             $absolute = true;
diff --git a/program/lib/Roundcube/rcube_utils.php b/program/lib/Roundcube/rcube_utils.php
index 842f677..063296d 100644
--- a/program/lib/Roundcube/rcube_utils.php
+++ b/program/lib/Roundcube/rcube_utils.php
@@ -1090,25 +1090,31 @@
     }
 
     /**
-     * Generate a ramdom string
+     * Generate a random string
      *
      * @param int  $length String length
-     * @param bool $raw    Return RAW data instead of hex
+     * @param bool $raw    Return RAW data instead of ascii
      *
      * @return string The generated random string
      */
     public static function random_bytes($length, $raw = false)
     {
-        $rlen   = $raw ? $length : ceil($length / 2);
-        $random = openssl_random_pseudo_bytes($rlen);
+        // Use PHP7 true random generator
+        if (function_exists('random_bytes')) {
+            $random = @random_bytes($length);
+        }
+
+        if (!$random) {
+            $random = openssl_random_pseudo_bytes($length);
+        }
 
         if ($raw) {
             return $random;
         }
 
-        $random = bin2hex($random);
+        $random = self::bin2ascii($random);
 
-        // if the length wasn't even...
+        // truncate to the specified size...
         if ($length < strlen($random)) {
             $random = substr($random, 0, $length);
         }
@@ -1117,6 +1123,50 @@
     }
 
     /**
+     * Convert binary data into readable form (containing a-zA-Z0-9 characters)
+     *
+     * @param string $input Binary input
+     *
+     * @return string Readable output
+     */
+    public static function bin2ascii($input)
+    {
+        // Above method returns "hexits".
+        // Based on bin_to_readable() function in ext/session/session.c.
+        // Note: removed ",-" characters from hextab
+        $hextab = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
+        $nbits  = 6; // can be 4, 5 or 6
+        $length = strlen($input);
+        $result = '';
+        $char   = 0;
+        $i      = 0;
+        $have   = 0;
+        $mask   = (1 << $nbits) - 1;
+
+        while (true) {
+            if ($have < $nbits) {
+                if ($i < $length) {
+                    $char |= ord($input[$i++]) << $have;
+                    $have += 8;
+                }
+                else if (!$have) {
+                    break;
+                }
+                else {
+                    $have = $nbits;
+                }
+            }
+
+            // consume nbits
+            $result .= $hextab[$char & $mask];
+            $char  >>= $nbits;
+            $have   -= $nbits;
+        }
+
+        return $result;
+    }
+
+    /**
      * Format current date according to specified format.
      * This method supports microseconds (u).
      *
diff --git a/tests/Framework/Utils.php b/tests/Framework/Utils.php
index dd0ff72..b0bfeef 100644
--- a/tests/Framework/Utils.php
+++ b/tests/Framework/Utils.php
@@ -430,7 +430,7 @@
      */
     function test_random_bytes()
     {
-        $this->assertSame(15, strlen(rcube_utils::random_bytes(15)));
+        $this->assertRegexp('/^[a-zA-Z0-9]{15}$/', rcube_utils::random_bytes(15));
         $this->assertSame(15, strlen(rcube_utils::random_bytes(15, true)));
         $this->assertSame(1, strlen(rcube_utils::random_bytes(1)));
         $this->assertSame(0, strlen(rcube_utils::random_bytes(0)));

--
Gitblit v1.9.1