PHP 7 - type declarations, use secure pseudo randoms

src/private/php/Auth.php

@@ -89,6 +89,7 @@ class Auth {
      * @param $poke bool|null true = poke user, false = send a message, null = default value from config
      * @return string|null|false Returns code as string on success, null when
      *         client cannot be found and false when other error occurs.
+     * @throws \TeamSpeak3_Adapter_ServerQuery_Exception
      */
     public static function generateConfirmationCode($cldbid, $poke = null) {
         if ($poke === null) {
@@ -98,7 +99,7 @@ class Auth {
         if (TeamSpeakUtils::i()->checkTSConnection()) {
             try {
                 $client = TeamSpeakUtils::i()->getTSNodeServer()->clientGetByDbid($cldbid);
-                $code = (string) mt_rand(100000, 999999); // TODO: replace it with a CSPRNG
+                $code = (string) Utils::getSecureRandomInt(100000, 999999);
                 $msg = LanguageUtils::tl("LOGIN_CONFIRMATION_CODE", $code);
 
                 if ($poke) {
@@ -113,6 +114,10 @@ class Auth {
                 if ($e->getCode() === 512) {
                     return null;
                 }
+
+                throw $e;
+            } catch (\Exception $e) {
+                // ignore exceptions from Utils::getSecureRandomInt
             }
         }
 
@@ -225,7 +230,8 @@ class Auth {
         }
 
         // Check if we data is already cached and if we can use it
-        if (isset($_SESSION["tsuser"]["servergroups"])) {
+        // no caching in dev mode
+        if (isset($_SESSION["tsuser"]["servergroups"]) && !__DEV_MODE) {
             $cached = $_SESSION["tsuser"]["servergroups"];
 
             // Calculate how old is the cached data (in seconds)
@@ -253,7 +259,7 @@ class Auth {
         }
 
         // Since the array in indexed with server group ID's, we can just separate the keys
-        // That gives us an array with ID's if user groups
+        // That gives us an array with ID's of user groups
         $serverGroupIds = array_keys($serverGroups);
 
         // Cache it in session with current time for later cachebusting

src/private/php/Utils/CsrfUtils.php

@@ -2,29 +2,18 @@
 
 namespace Wruczek\TSWebsite\Utils;
 
-
 class CsrfUtils {
 
+    public const CSRF_TOKEN_BYTES_LENGTH = 32;
+
     /**
      * Generates and returns a new CSRF token
-     * @param $length int length in bytes
+     * @param $bytes int length in bytes
      * @return string generated CSRF token
      * @throws \Exception when unable to generate a new token
      */
-    public static function generateToken($length) {
+    public static function generateToken(int $bytes): string {
-        if (function_exists("random_bytes")) {
+        return Utils::getSecureRandomString($bytes);
-            $token = bin2hex(random_bytes($length));
-        } else if (function_exists("mcrypt_create_iv")) {
-            $token = bin2hex(mcrypt_create_iv($length, MCRYPT_DEV_URANDOM));
-        } else {
-            $token = bin2hex(openssl_random_pseudo_bytes($length));
-        }
-
-        if (!is_string($token) || empty($token)) {
-            throw new \Exception("Cannot generate new CSRF token");
-        }
-
-        return $token;
     }
 
     /**
@@ -32,13 +21,12 @@ class CsrfUtils {
      * @return string CSRF token
      * @throws \Exception When we cannot generate a new CSRF token
      */
-    public static function getToken() {
+    public static function getToken(): string {
         if (isset($_SESSION["csrfToken"])) {
             return $_SESSION["csrfToken"];
         }
 
-        $length = 16; // in bytes
+        $token = self::generateToken(self::CSRF_TOKEN_BYTES_LENGTH);
-        $token = self::generateToken($length);
 
         $_SESSION["csrfToken"] = $token;
         return $token;
@@ -49,7 +37,7 @@ class CsrfUtils {
      * @param $toCheck string token to be checked
      * @return bool true if tokens match, false otherwise.
      */
-    public static function validateToken($toCheck) {
+    public static function validateToken(string $toCheck): bool {
         $knownToken = @$_SESSION["csrfToken"];
 
         if ($knownToken === null) {
@@ -63,7 +51,7 @@ class CsrfUtils {
      * Tries to get CSRF token from the request and then compares it.
      * If it fails, it returns the error page with message and exits the script.
      */
-    public static function validateRequest() {
+    public static function validateRequest(): void {
         if (isset($_POST["csrf-token"])) {
             $csrfToken = $_POST["csrf-token"];
         } else if (isset($_SERVER["HTTP_X_CSRF_TOKEN"])) {

src/private/php/Utils/TeamSpeakUtils.php

@@ -2,8 +2,6 @@
 
 namespace Wruczek\TSWebsite\Utils;
 
-use function mt_rand;
-use function var_dump;
 use Wruczek\TSWebsite\Config;
 
 /**

src/private/php/Utils/Utils.php

@@ -20,8 +20,8 @@ class Utils {
      * @param $string string String to be escaped
      * @return string escaped string
      */
-    public static function escape($string) {
+    public static function escape(string $string): string {
-        return htmlspecialchars((string) $string, ENT_QUOTES, "UTF-8");
+        return htmlspecialchars($string, ENT_QUOTES, "UTF-8");
     }
 
     /**
@@ -30,7 +30,7 @@ class Utils {
      * @param $str
      * @return bool|string stripped text without the first line or false on failure
      */
-    public static function stripFirstLine($str) {
+    public static function stripFirstLine(string $str) {
         $position = strpos($str, "\n");
 
         if($position === false)
@@ -47,7 +47,7 @@ class Utils {
      * @param bool $case set to false for case-insensitivity (default true)
      * @return bool true if $haystack starts with $needle, false otherwise
      */
-    public static function startsWith($haystack, $needle, $case = true) {
+    public static function startsWith(string $haystack, string $needle, bool $case = true): bool {
         if ($case)
             return strpos($haystack, $needle, 0) === 0;
 
@@ -62,7 +62,7 @@ class Utils {
      * @param bool $case set to false for case-insensitivity (default true)
      * @return bool true if $haystack ends with $needle, false otherwise
      */
-    public static function endsWith($haystack, $needle, $case = true) {
+    public static function endsWith(string $haystack, string $needle, bool $case = true): bool {
         $expectedPosition = strlen($haystack) - strlen($needle);
 
         if ($case)
@@ -77,7 +77,7 @@ class Utils {
      * @return bool|string Censored IP on success, false on failure
      * @throws \Exception When the IP address is invalid
      */
-    public static function censorIpAddress($ip) {
+    public static function censorIpAddress(string $ip): string {
         if (filter_var($ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) {
             $ip = explode(".", $ip);
 
@@ -107,7 +107,7 @@ class Utils {
      *             Falls back to REMOTE_ADDR if empty
      * @return string IP address
      */
-    public static function getClientIp($useCfip = null) {
+    public static function getClientIp(bool $useCfip = null): string {
         if ($useCfip === null) {
             $useCfip = Config::get("usingcloudflare");
         }
@@ -128,7 +128,7 @@ class Utils {
      * Returns currently used news store
      * @return INewsStore|null
      */
-    public static function getNewsStore() {
+    public static function getNewsStore(): ?INewsStore {
         $newsStore = null;
 
         // if the current implementation is default
@@ -138,4 +138,28 @@ class Utils {
 
         return $newsStore;
     }
+
+    /**
+     * Generate secure random int in specified bounds
+     * @see \random_int()
+     * @param int $min
+     * @param int $max
+     * @return int
+     * @throws \Exception
+     */
+    public static function getSecureRandomInt(int $min, int $max): int {
+        return \random_int($min, $max);
+    }
+
+    /**
+     * Generate secure random bytes and convert them into hex
+     * @see \random_bytes()
+     * @see \bin2hex()
+     * @param int $bytes
+     * @return string
+     * @throws \Exception
+     */
+    public static function getSecureRandomString(int $bytes): string {
+        return \bin2hex(\random_bytes($bytes));
+    }
 }