security: SQL Injection

This mitigates a possible SQL injection vulnerability reported by Bittylicious.
osTicket · Oct 7, 2021 · e282910 · e282910
1 parent e90d3be
commit e282910
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 12 deletions.
diff --git a/include/class.staff.php b/include/class.staff.php
@@ -862,11 +862,11 @@ static function lookup($var) {
         if (is_array($var))
             return parent::lookup($var);
         elseif (is_numeric($var))
-            return parent::lookup(array('staff_id'=>$var));
+            return parent::lookup(array('staff_id' => (int) $var));
         elseif (Validator::is_email($var))
-            return parent::lookup(array('email'=>$var));
-        elseif (is_string($var))
-            return parent::lookup(array('username'=>$var));
+            return parent::lookup(array('email' => $var));
+        elseif (is_string($var) &&  Validator::is_username($var))
+            return parent::lookup(array('username' => (string) $var));
         else
             return null;
     }

diff --git a/include/class.user.php b/include/class.user.php
@@ -1341,10 +1341,10 @@ static function createForUser($user, $defaults=false) {
     }
 
     static function lookupByUsername($username) {
-        if (strpos($username, '@') !== false)
-            $user = static::lookup(array('user__emails__address'=>$username));
-        else
-            $user = static::lookup(array('username'=>$username));
+        if (Validator::is_email($username))
+            $user = static::lookup(array('user__emails__address' => $username));
+        elseif (Validator::is_userid($username))
+            $user = static::lookup(array('username' => $username));
 
         return $user;
     }

diff --git a/include/class.validator.php b/include/class.validator.php
@@ -220,11 +220,18 @@ static function is_ip($ip, &$error='') {
     static function is_username($username, &$error='') {
         if (strlen($username)<2)
             $error = __('Username must have at least two (2) characters');
-        elseif (!preg_match('/^[\p{L}\d._-]+$/u', $username))
+        elseif (is_numeric($username) || !preg_match('/^[\p{L}\d._-]+$/u', $username))
             $error = __('Username contains invalid characters');
         return $error == '';
     }
 
+    static  function is_userid($userid, &$error='') {
+        if (!self::is_username($userid)
+                    && !self::is_email($userid))
+            $error = __('Invalid User Id ');
+        return $error == '';
+    }
+
     static function is_formula($text, &$error='') {
         if (!preg_match('/^[^=\+@-].*$/s', $text))
             $error = __('Content cannot start with the following characters: = - + @');

diff --git a/pwreset.php b/pwreset.php
@@ -15,7 +15,9 @@
     }
     switch ($_POST['do']) {
         case 'sendmail':
-            if (($acct=ClientAccount::lookupByUsername($_POST['userid']))) {
+            $userid = (string) $_POST['userid'];
+            if (Validator::is_userid($userid)
+                    && ($acct=ClientAccount::lookupByUsername($userid))) {
                 if (!$acct->isPasswdResetEnabled()) {
                     $banner = __('Password reset is not enabled for your account. Contact your administrator');
                 }

diff --git a/scp/pwreset.php b/scp/pwreset.php
@@ -39,7 +39,9 @@
     }
     switch ($_POST['do']) {
         case 'sendmail':
-            if (($staff=Staff::lookup($_POST['userid']))) {
+            $userid = (string) $_POST['userid'];
+            if (Validator::is_userid($userid)
+                    && ($staff=Staff::lookup($userid))) {
                 if (!$staff->hasPassword()) {
                     if ($staff->sendResetEmail('registration-staff', false) !== false)
                         $msg = __('Registration email sent successfully.');
@@ -72,7 +74,8 @@
     $msg = __('Please enter your username or email');
     $_config = new Config('pwreset');
     if (($id = $_config->get($_GET['token']))
-            && ($staff = Staff::lookup($id)))
+            && is_numeric($id)
+            && ($staff = Staff::lookup( (int) $id)))
         // TODO: Detect staff confirmation (for welcome email)
         $tpl = 'pwreset.login.php';
     else