CVE-2021-42235: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection') (CWE-89)
value: SQL injection in osTicket before 1.14.8 and 1.15.4 login and password reset process allows attackers to access the osTicket administration profile functionality.
CWE description: The product constructs all or part of an SQL command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended SQL command when it is sent to a downstream component. Without sufficient removal or quoting of SQL syntax in user-controllable inputs, the generated SQL query can cause those inputs to be interpreted as SQL instead of ordinary user data.
CVSS v3 base score: 9.8
Published: 2022-05-04T17:15Z
Repository: https://github.com/osTicket/osTicket
Language: PHP
Files changed: include/class.staff.php, include/class.user.php, include/class.validator.php, pwreset.php, scp/pwreset.php
Commit date: 2021-10-07 14:38:44 +0000
Commit message: security: SQL Injection

This mitigates a possible SQL injection vulnerability reported by
Bittylicious.

Diff: diff --git a/include/class.staff.php b/include/class.staff.php
index 0c16edcd38..7c9b2afc5e 100644
--- a/include/class.staff.php
+++ b/include/class.staff.php
@@ -860,15 +860,15 @@ implements AuthenticatedUser, EmailContact, TemplateVariable, Searchable {
     /**** Static functions ********/
     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;
     }
 
     static function getStaffMembers($criteria=array()) {
diff --git a/include/class.user.php b/include/class.user.php
index e99262464b..af9a353e67 100644
--- a/include/class.user.php
+++ b/include/class.user.php
@@ -1339,14 +1339,14 @@ class UserAccount extends VerySimpleModel {
         }
         return $acct;
     }
 
     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;
     }
 
     static function register($user, $vars, &$errors) {
diff --git a/include/class.validator.php b/include/class.validator.php
index aad6b53920..8b4db6a77b 100644
--- a/include/class.validator.php
+++ b/include/class.validator.php
@@ -218,15 +218,22 @@ class Validator {
     }
 
     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: = - + @');
         return $error == '';
     }
diff --git a/pwreset.php b/pwreset.php
index bc9ef6f29d..b2c42a6a79 100644
--- a/pwreset.php
+++ b/pwreset.php
@@ -13,11 +13,13 @@ if($_POST) {
         Http::response(400, __('Valid CSRF Token Required'));
         exit;
     }
     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');
                 }
                 elseif ($acct->sendResetEmail()) {
                     $inc = 'pwreset.sent.php';
diff --git a/scp/pwreset.php b/scp/pwreset.php
index a81689d24e..10ce4fd76a 100644
--- a/scp/pwreset.php
+++ b/scp/pwreset.php
@@ -37,11 +37,13 @@ if($_POST) {
         Http::response(400, __('Valid CSRF Token Required'));
         exit;
     }
     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.');
                     else
                         $msg = __('Unable to reset password. Contact your administrator');
@@ -70,11 +72,12 @@ if($_POST) {
 }
 elseif ($_GET['token']) {
     $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
         header('Location: index.php');
 }

Vulnerable code:             return parent::lookup(array('staff_id'=>$var));
            return parent::lookup(array('email'=>$var));
        elseif (is_string($var))
            return parent::lookup(array('username'=>$var));
        if (strpos($username, '@') !== false)
            $user = static::lookup(array('user__emails__address'=>$username));
        else
            $user = static::lookup(array('username'=>$username));
        elseif (!preg_match('/^[\p{L}\d._-]+$/u', $username))
            if (($acct=ClientAccount::lookupByUsername($_POST['userid']))) {
            if (($staff=Staff::lookup($_POST['userid']))) {
            && ($staff = Staff::lookup($id)))
Fixed code:             return parent::lookup(array('staff_id' => (int) $var));
            return parent::lookup(array('email' => $var));
        elseif (is_string($var) &&  Validator::is_username($var))
            return parent::lookup(array('username' => (string) $var));
        if (Validator::is_email($username))
            $user = static::lookup(array('user__emails__address' => $username));
        elseif (Validator::is_userid($username))
            $user = static::lookup(array('username' => $username));
        elseif (is_numeric($username) || !preg_match('/^[\p{L}\d._-]+$/u', $username))
    static  function is_userid($userid, &$error='') {
        if (!self::is_username($userid)
                    && !self::is_email($userid))
            $error = __('Invalid User Id ');
        return $error == '';
    }

            $userid = (string) $_POST['userid'];
            if (Validator::is_userid($userid)
                    && ($acct=ClientAccount::lookupByUsername($userid))) {
            $userid = (string) $_POST['userid'];
            if (Validator::is_userid($userid)
                    && ($staff=Staff::lookup($userid))) {
            && is_numeric($id)
            && ($staff = Staff::lookup( (int) $id)))