Fix now invalid usages of CryptoException (#2908)
authorJens Hausdorf <mail@jens-hausdorf.de>
Mon, 17 Jun 2019 16:24:49 +0000 (18:24 +0200)
committerAlexander Ebert <ebert@woltlab.com>
Mon, 17 Jun 2019 16:24:49 +0000 (18:24 +0200)
* Fix now invalid usages of CryptoException

The \random_{bytes,int} functions are now called directly instead through
wcf\util\CryptoUtil,  which means that the exceptions may not be caught
with CryptoException anymore. Instead, they must be caught with the
generic \Throwable interface.
This commit also fixes usages of those functions to prevent malicious code
to overwrite these critical functions in userland code.

* Make sure to generate a truly random ID
by disallowing user modification.

Adding the `\` in front of the function call makes sure PHP does not use
any overwritten (user-land) function, potentially destroying our efforts of
having a CSRPNG ID

* Remove no longer necessary try-catch block.

wcfsetup/install/files/lib/system/package/PackageInstallationDispatcher.class.php
wcfsetup/install/files/lib/system/worker/SendNewPasswordWorker.class.php
wcfsetup/install/files/lib/util/CryptoUtil.class.php
wcfsetup/install/files/lib/util/StringUtil.class.php

index 1e3e0cc63ec2adf9092a0579009d8f00e794b07b..8ea75049bba00925f65202b4f6e4ba163b8d1aa3 100644 (file)
@@ -36,7 +36,6 @@ use wcf\system\setup\Installer;
 use wcf\system\style\StyleHandler;
 use wcf\system\user\storage\UserStorageHandler;
 use wcf\system\WCF;
-use wcf\util\exception\CryptoException;
 use wcf\util\FileUtil;
 use wcf\util\HeaderUtil;
 use wcf\util\StringUtil;
@@ -227,7 +226,7 @@ class PackageInstallationDispatcher {
                                                        'signature_secret'
                                                ]);
                                        }
-                                       catch (CryptoException $e) {
+                                       catch (\Throwable $e) {
                                                // ignore, the secret will stay empty and crypto operations
                                                // depending on it will fail
                                        }
index 0f2f4340c4012f2492ab75a28844329791f2d5bd..87c93fb92eb12754d464839640b00cc1cfebb0ea 100644 (file)
@@ -13,7 +13,6 @@ use wcf\system\email\UserMailbox;
 use wcf\system\exception\SystemException;
 use wcf\system\request\LinkHandler;
 use wcf\system\WCF;
-use wcf\util\exception\CryptoException;
 
 /**
  * Worker implementation for sending new passwords.
@@ -94,14 +93,8 @@ class SendNewPasswordWorker extends AbstractWorker {
         * @param       UserEditor      $userEditor
         */
        protected function resetPassword(UserEditor $userEditor) {
-               try {
-                       $lostPasswordKey = bin2hex(\random_bytes(20));
-                       $lastLostPasswordRequestTime = TIME_NOW;
-               }
-               catch (CryptoException $e) {
-                       $lostPasswordKey = null;
-                       $lastLostPasswordRequestTime = 0;
-               }
+               $lostPasswordKey = bin2hex(\random_bytes(20));
+               $lastLostPasswordRequestTime = TIME_NOW;
                $userAction = new UserAction([$userEditor], 'update', [
                        'data' => [
                                'password' => null,
index 570333338d5fbfc42a9c5c2c22dee78af08b5a16..0856d7510d6e8d2e59f2c079c886bdab8fce5cea 100644 (file)
@@ -84,7 +84,7 @@ final class CryptoUtil {
         * @deprecated  Use \random_bytes() directly.
         */
        public static function randomBytes($n) {
-               return random_bytes($n);
+               return \random_bytes($n);
        }
        
        /**
@@ -97,7 +97,7 @@ final class CryptoUtil {
                        throw new CryptoException("Cannot generate a secure random number, min and max are the same");
                }
 
-               return random_int($min, $max);
+               return \random_int($min, $max);
        }
        
        /**
index f96a279925c238f9861935bc180c29e16c9b8a10..74e232055fc412017e308f1cb12aec9d280ee9d6 100644 (file)
@@ -47,7 +47,7 @@ final class StringUtil {
         * @return      string
         */
        public static function getRandomID() {
-               return bin2hex(random_bytes(20));
+               return \bin2hex(\random_bytes(20));
        }
        
        /**
@@ -59,17 +59,17 @@ final class StringUtil {
                return sprintf(
                        '%04x%04x-%04x-%04x-%02x%02x-%04x%04x%04x',
                        // time_low
-                       random_int(0, 0xffff), random_int(0, 0xffff),
+                       \random_int(0, 0xffff), \random_int(0, 0xffff),
                        // time_mid
-                       random_int(0, 0xffff), 
+                       \random_int(0, 0xffff), 
                        // time_hi_and_version
-                       random_int(0, 0x0fff) | 0x4000,
+                       \random_int(0, 0x0fff) | 0x4000,
                        // clock_seq_hi_and_res
-                       random_int(0, 0x3f) | 0x80,
+                       \random_int(0, 0x3f) | 0x80,
                        // clock_seq_low
-                       random_int(0, 0xff),
+                       \random_int(0, 0xff),
                        // node
-                       random_int(0, 0xffff), random_int(0, 0xffff), random_int(0, 0xffff)
+                       \random_int(0, 0xffff), \random_int(0, 0xffff), \random_int(0, 0xffff)
                );
        }