CVE-2016-7400: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection') (CWE-89)
value: Multiple SQL injection vulnerabilities in Exponent CMS before 2.4.0 allow remote attackers to execute arbitrary SQL commands via the (1) id parameter in an activate_address address controller action, (2) title parameter in a show blog controller action, or (3) content_id parameter in a showComments expComment controller action.
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: 2017-02-07T15:59Z
Repository: https://github.com/exponentcms/exponent-cms
Language: PHP
Files changed: framework/core/subsystems/database/mysqli.php, framework/core/subsystems/expDatabase.php, framework/core/subsystems/expRouter.php, framework/core/subsystems/expString.php, framework/core/subsystems/expTheme.php
Commit date: 2016-09-13 15:38:24 -0400
Commit message: security fix several sql injection vulnerabilities reported by Manuel Garcia Cardenas and PKAV TEAM

Diff: diff --git a/framework/core/subsystems/database/mysqli.php b/framework/core/subsystems/database/mysqli.php
index 951e4ac88..6a323b561 100644
--- a/framework/core/subsystems/database/mysqli.php
+++ b/framework/core/subsystems/database/mysqli.php
@@ -377,10 +377,12 @@ class mysqli_database extends database {
      * @return array
      */
     function selectObjects($table, $where = null, $orderby = null) {
         if ($where == null)
             $where = "1";
+        else
+            $where = $this->injectProof($where);
         if ($orderby == null)
             $orderby = '';
         else
             $orderby = "ORDER BY " . $orderby;
 
@@ -482,11 +484,11 @@ class mysqli_database extends database {
     function selectObjectBySql($sql) {
         //$logFile = "C:\\xampp\\htdocs\\supserg\\tmp\\queryLog.txt";
         //$lfh = fopen($logFile, 'a');
         //fwrite($lfh, $sql . "\n");    
         //fclose($lfh);                 
-        $res = @mysqli_query($this->connection, $sql);
+        $res = @mysqli_query($this->connection, $this->injectProof($sql));
         if ($res == null)
             return null;
         return mysqli_fetch_object($res);
     }
 
@@ -495,11 +497,11 @@ class mysqli_database extends database {
      *
 	 * @param  $sql
 	 * @return array
 	 */
     function selectObjectsBySql($sql) {
-        $res = @mysqli_query($this->connection, $sql);
+        $res = @mysqli_query($this->connection, $this->injectProof($sql));
         if ($res == null)
             return array();
         $objects = array();
         for ($i = 0, $iMax = mysqli_num_rows($res); $i < $iMax; $i++)
             $objects[] = mysqli_fetch_object($res);
@@ -636,10 +638,12 @@ class mysqli_database extends database {
      * @return array
      */
     function selectObjectsIndexedArray($table, $where = null, $orderby = null) {
         if ($where == null)
             $where = "1";
+        else
+            $where = $this->injectProof($where);
         if ($orderby == null)
             $orderby = '';
         else
             $orderby = "ORDER BY " . $orderby;
         $res = @mysqli_query($this->connection, "SELECT * FROM `" . $this->prefix . "$table` WHERE $where $orderby");
@@ -720,10 +724,11 @@ class mysqli_database extends database {
      * @param string $table The name of the table/object to look at
      * @param string $where Criteria used to narrow the result set.
      * @return object/null|void
      */
     function selectObject($table, $where) {
+        $where = $this->injectProof($where);
         $res = mysqli_query($this->connection, "SELECT * FROM `" . $this->prefix . "$table` WHERE $where LIMIT 0,1");
         if ($res == null)
             return null;
         return mysqli_fetch_object($res);
     }
@@ -771,11 +776,11 @@ class mysqli_database extends database {
             if ($var{0} != '_') {
                 $sql .= "`$var`,";
                 if ($values != ") VALUES (") {
                     $values .= ",";
                 }
-                $values .= "'" . mysqli_real_escape_string($this->connection, $val) . "'";
+                $values .= "'" . $this->escapeString($val) . "'";
             }
         }
         $sql = substr($sql, 0, -1) . substr($values, 0) . ")";
         //if($table=='text')eDebug($sql,true);
         if (@mysqli_query($this->connection, $sql) != false) {
@@ -834,17 +839,17 @@ class mysqli_database extends database {
             if ($var{0} != '_') {
                 if (is_array($val) || is_object($val)) {
                     $val = serialize($val);   
                     $sql .= "`$var`='".$val."',";
                 } else {
-                    $sql .= "`$var`='".mysqli_real_escape_string($this->connection,$val)."',";
+                    $sql .= "`$var`='" . $this->escapeString($val) . "',";
                 }
             }
         }
         $sql = substr($sql, 0, -1) . " WHERE ";
         if ($where != null)
-            $sql .= $where;
+            $sql .= $this->injectProof($where);
         else
             $sql .= "`" . $identifier . "`=" . $object->$identifier;
         //if ($table == 'text') eDebug($sql,true);        
         $res = (@mysqli_query($this->connection, $sql) != false);
         return $res;
@@ -1128,10 +1133,12 @@ class mysqli_database extends database {
      * @return array
      */
     function selectArrays($table, $where = null, $orderby = null) {
         if ($where == null)
             $where = "1";
+        else
+            $where = $this->injectProof($where);
         if ($orderby == null)
             $orderby = '';
         else
             $orderby = "ORDER BY " . $orderby;
 
@@ -1154,11 +1161,11 @@ class mysqli_database extends database {
      *
      * @param string $sql The name of the table/object to look at
      * @return array
      */
     function selectArraysBySql($sql) {        
-        $res = @mysqli_query($this->connection, $sql);
+        $res = @mysqli_query($this->connection, $this->injectProof($sql));
         if ($res == null)
             return array();
         $arrays = array();
         for ($i = 0, $iMax = mysqli_num_rows($res); $i < $iMax; $i++)
             $arrays[] = mysqli_fetch_assoc($res);
@@ -1183,10 +1190,12 @@ class mysqli_database extends database {
      * @return array|void
      */
     function selectArray($table, $where = null, $orderby = null, $is_revisioned=false, $needs_approval=false) {
         if ($where == null)
             $where = "1";
+        else
+            $where = $this->injectProof($where);
         $as = '';
         if ($is_revisioned) {
    //            $where.= " AND revision_id=(SELECT MAX(revision_id) FROM `" . $this->prefix . "$table` WHERE $where)";
             $where .= " AND revision_id=(SELECT MAX(revision_id) FROM `" . $this->prefix . "$table` WHERE id = rev.id ";
             if ($needs_approval) $where .= ' AND (approved=1 OR poster=' . $needs_approval . ' OR editor=' . $needs_approval . ')';
@@ -1221,10 +1230,12 @@ class mysqli_database extends database {
      * @return array
      */
     function selectExpObjects($table, $where=null, $classname, $get_assoc=true, $get_attached=true, $except=array(), $cascade_except=false, $order=null, $limitsql=null, $is_revisioned=false, $needs_approval=false) {
         if ($where == null)
             $where = "1";
+        else
+            $where = $this->injectProof($where);
         $as = '';
         if ($is_revisioned) {
    //            $where.= " AND revision_id=(SELECT MAX(revision_id) FROM `" . $this->prefix . "$table` WHERE $where)";
             $where .= " AND revision_id=(SELECT MAX(revision_id) FROM `" . $this->prefix . "$table` WHERE id = rev.id ";
             if ($needs_approval) $where .= ' AND (approved=1 OR poster=' . $needs_approval . ' OR editor=' . $needs_approval . ')';
@@ -1257,11 +1268,11 @@ class mysqli_database extends database {
      * @param bool $get_assoc
      * @param bool $get_attached
      * @return array
      */
     function selectExpObjectsBySql($sql, $classname, $get_assoc=true, $get_attached=true) {
-        $res = @mysqli_query($this->connection, $sql);
+        $res = @mysqli_query($this->connection, $this->injectProof($sql));
         if ($res == null)
             return array();
         $arrays = array();
         $numrows = mysqli_num_rows($res);
         for ($i = 0; $i < $numrows; $i++)
diff --git a/framework/core/subsystems/expDatabase.php b/framework/core/subsystems/expDatabase.php
index d69406f90..50630c366 100644
--- a/framework/core/subsystems/expDatabase.php
+++ b/framework/core/subsystems/expDatabase.php
@@ -1103,10 +1103,25 @@ abstract class database {
 	 * @param $string
 	 * @return string
 	 */
 	abstract function escapeString($string);
 
+    /**
+   	 * Unescape a string based on the database connection
+   	 * @param $string
+   	 * @return string
+   	 */
+   	function injectProof($string) {
+   	    $quotes = substr_count("'", $string);
+        if ($quotes % 2 != 0)
+            $string = $this->escapeString($string);
+        $dquotes = substr_count('"', $string);
+        if ($dquotes % 2 != 0)
+            $string = $this->escapeString($string);
+        return $string;
+    }
+
 	/**
 	 * Create a SQL "limit" phrase
 	 *
 	 * @param  $num
 	 * @param  $offset
diff --git a/framework/core/subsystems/expRouter.php b/framework/core/subsystems/expRouter.php
index 2ad5cdf2a..5c2e72e32 100644
--- a/framework/core/subsystems/expRouter.php
+++ b/framework/core/subsystems/expRouter.php
@@ -196,15 +196,30 @@ class expRouter {
                     $_GET[$key],
                     $_REQUEST[$key]
                 );
             }
         }
-        // conventional method to ensure the 'id' is an id
-        if (isset($_GET['id'])) {
-            $_GET['id'] = intval($_GET['id']);
+        // conventional method to ensure the 'id' is only an id
+        if (isset($_REQUEST['id'])) {
+            if (isset($_GET['id']))
+                $_GET['id'] = intval($_GET['id']);
+            if (isset($_POST['id']))
+                $_POST['id'] = intval($_POST['id']);
+
             $_REQUEST['id'] = intval($_REQUEST['id']);
         }
+        // do the same for the other id's
+        foreach ($_REQUEST as $key=>$var) {
+            if (is_string($var) && strrpos($key,'_id',-3) !== false) {
+                if (isset($_GET[$key]))
+                    $_GET[$key] = intval($_GET[$key]);
+                if (isset($_POST[$key]))
+                    $_POST[$key] = intval($_POST[$key]);
+
+                $_REQUEST[$key] = intval($_REQUEST[$key]);
+            }
+        }
         if (empty($user->id) || (!empty($user->id) && !$user->isAdmin())) {  //FIXME why would $user be empty here unless $db is down?
 //            $_REQUEST['route_sanitized'] = true;//FIXME debug test
             expString::sanitize($_REQUEST);  // strip other exploits like sql injections
         }
 
@@ -305,18 +320,18 @@ class expRouter {
             if ($this->url_parts[count($this->url_parts)-1] == '') array_pop($this->url_parts);
             if (empty($this->url_parts[0])) array_shift($this->url_parts);
             
             if (count($this->url_parts) < 1 || (empty($this->url_parts[0]) && count($this->url_parts) == 1) ) {
                 $this->url_type = 'base';  // no params
-            } elseif (count($this->url_parts) == 1 || $db->selectObject('section', "sef_name='" . substr($db->escapeString($this->sefPath),1) . "'") != null) {
+            } elseif (count($this->url_parts) == 1 || $db->selectObject('section', "sef_name='" . substr($this->sefPath,1) . "'") != null) {
                 $this->url_type = 'page';  // single param is page name
             } elseif ($_SERVER['REQUEST_METHOD'] == 'POST') {
                 $this->url_type = 'post';  // params via form/post
             } else {
                 // take a peek and see if a page exists with the same name as the first value...if so we probably have a page with
                 // extra perms...like printerfriendly=1 or ajax_action=1;
-                if (($db->selectObject('section', "sef_name='" . $db->escapeString($this->url_parts[0]) . "'") != null) && (in_array(array('printerfriendly','exportaspdf','ajax_action'), $this->url_parts))) {
+                if (($db->selectObject('section', "sef_name='" . $this->url_parts[0] . "'") != null) && (in_array(array('printerfriendly','exportaspdf','ajax_action'), $this->url_parts))) {
                     $this->url_type = 'page';
                 } else {
                     $this->url_type = 'action';
                 }
             }
@@ -545,11 +560,11 @@ class expRouter {
         if ($this->url_style == 'sef') {
             $url .= substr(PATH_RELATIVE,0,-1).$this->sefPath;
         } else {
             $url .= urldecode((empty($_SERVER['REQUEST_URI'])) ? $_ENV['REQUEST_URI'] : $_SERVER['REQUEST_URI']);
         }
-        return expString::sanitize($url);
+        return expString::escape(expString::sanitize($url));
     }
 
     public static function encode($url) {
         $url = str_replace('&', 'and', $url);
         return preg_replace("/(-)$/", "", preg_replace('/(-){2,}/', '-', strtolower(preg_replace("/([^0-9a-z-_\+])/i", '-', $url))));
@@ -758,11 +773,11 @@ class expRouter {
             }            
         }
         if (substr($this->sefPath,-1) == "/") $this->sefPath = substr($this->sefPath,0,-1);
         // sanitize it
         $sefPath = explode('">',$this->sefPath);  // remove any attempts to close the command
-        $this->sefPath = expString::sanitize($sefPath[0]);
+        $this->sefPath = expString::escape(expString::sanitize($sefPath[0]));
     }
 
     public function getSection() {
         global $db;
 
diff --git a/framework/core/subsystems/expString.php b/framework/core/subsystems/expString.php
index abc0f0264..57901dcc3 100644
--- a/framework/core/subsystems/expString.php
+++ b/framework/core/subsystems/expString.php
@@ -520,27 +520,27 @@ class expString {
             }
 
             $data = self::xss_clean($data);
 
             //fixme orig exp method
-            if(0) {
-                // remove whitespaces and tags
-//            $data = strip_tags(trim($data));
-                // remove whitespaces and script tags
-                $data = self::strip_tags_content(trim($data), '<script>', true);
-//            $data = self::strip_tags_content(trim($data), '<iframe>', true);
-
-                // apply stripslashes if magic_quotes_gpc is enabled
-                if (get_magic_quotes_gpc()) {
-                    $data = stripslashes($data);
-                }
-
-                $data = self::escape($data);
-
-                // re-escape newlines
-                $data = str_replace(array('\r', '\n'), array("\r", "\n"), $data);
-            }
+//            if(0) {
+//                // remove whitespaces and tags
+////            $data = strip_tags(trim($data));
+//                // remove whitespaces and script tags
+//                $data = self::strip_tags_content(trim($data), '<script>', true);
+////            $data = self::strip_tags_content(trim($data), '<iframe>', true);
+//
+//                // apply stripslashes if magic_quotes_gpc is enabled
+//                if (get_magic_quotes_gpc()) {
+//                    $data = stripslashes($data);
+//                }
+//
+//                $data = self::escape($data);
+//
+//                // re-escape newlines
+//                $data = str_replace(array('\r', '\n'), array("\r", "\n"), $data);
+//            }
         }
         return $data;
     }
 
     // xss_clean //
diff --git a/framework/core/subsystems/expTheme.php b/framework/core/subsystems/expTheme.php
index 83ee4f64e..617d7be1f 100755
--- a/framework/core/subsystems/expTheme.php
+++ b/framework/core/subsystems/expTheme.php
@@ -760,11 +760,11 @@ class expTheme
 //            }
 //            if (empty($_REQUEST['route_sanitized'])) {
             if (!$user->isAdmin())
                 expString::sanitize($_REQUEST);
 //            } elseif (empty($_REQUEST['array_sanitized'])) {
-                $tmp =1;  //FIXME we've already sanitized at this point
+//                $tmp =1;  //FIXME we've already sanitized at this point
 //            } else {
 //                $tmp =1;  //FIXME we've already sanitized at this point
 //            }
 
             //FIXME: module/controller glue code..remove ASAP
@@ -841,11 +841,12 @@ class expTheme
         if (isset($params)) {
 //            foreach ($params as $key => $value) {  //FIXME need array sanitizer
 ////                $_GET[$key] = $value;
 //                $_GET[$key] = expString::sanitize($value);
 //            }
-            if (!$user->isAdmin()) expString::sanitize($_GET);
+            if (!$user->isAdmin())
+                expString::sanitize($_GET);
         }
         //if (isset($['_common'])) $actfile = "/common/actions/" . $_REQUEST['action'] . ".php";
 
         if (is_readable(BASE . "themes/" . DISPLAY_THEME . "/modules" . $actfile)) {
             include(BASE . "themes/" . DISPLAY_THEME . "/modules" . $actfile);

Vulnerable code:         $res = @mysqli_query($this->connection, $sql);
        $res = @mysqli_query($this->connection, $sql);
                $values .= "'" . mysqli_real_escape_string($this->connection, $val) . "'";
                    $sql .= "`$var`='".mysqli_real_escape_string($this->connection,$val)."',";
            $sql .= $where;
        $res = @mysqli_query($this->connection, $sql);
        $res = @mysqli_query($this->connection, $sql);
        // conventional method to ensure the 'id' is an id
        if (isset($_GET['id'])) {
            $_GET['id'] = intval($_GET['id']);
            } elseif (count($this->url_parts) == 1 || $db->selectObject('section', "sef_name='" . substr($db->escapeString($this->sefPath),1) . "'") != null) {
                if (($db->selectObject('section', "sef_name='" . $db->escapeString($this->url_parts[0]) . "'") != null) && (in_array(array('printerfriendly','exportaspdf','ajax_action'), $this->url_parts))) {
        return expString::sanitize($url);
        $this->sefPath = expString::sanitize($sefPath[0]);
            if(0) {
                // remove whitespaces and tags
//            $data = strip_tags(trim($data));
                // remove whitespaces and script tags
                $data = self::strip_tags_content(trim($data), '<script>', true);
//            $data = self::strip_tags_content(trim($data), '<iframe>', true);

                // apply stripslashes if magic_quotes_gpc is enabled
                if (get_magic_quotes_gpc()) {
                    $data = stripslashes($data);
                }

                $data = self::escape($data);

                // re-escape newlines
                $data = str_replace(array('\r', '\n'), array("\r", "\n"), $data);
            }
                $tmp =1;  //FIXME we've already sanitized at this point
            if (!$user->isAdmin()) expString::sanitize($_GET);
Fixed code:         else
            $where = $this->injectProof($where);
        $res = @mysqli_query($this->connection, $this->injectProof($sql));
        $res = @mysqli_query($this->connection, $this->injectProof($sql));
        else
            $where = $this->injectProof($where);
        $where = $this->injectProof($where);
                $values .= "'" . $this->escapeString($val) . "'";
                    $sql .= "`$var`='" . $this->escapeString($val) . "',";
            $sql .= $this->injectProof($where);
        else
            $where = $this->injectProof($where);
        $res = @mysqli_query($this->connection, $this->injectProof($sql));
        else
            $where = $this->injectProof($where);
        else
            $where = $this->injectProof($where);
        $res = @mysqli_query($this->connection, $this->injectProof($sql));
    /**
   	 * Unescape a string based on the database connection
   	 * @param $string
   	 * @return string
   	 */
   	function injectProof($string) {
   	    $quotes = substr_count("'", $string);
        if ($quotes % 2 != 0)
            $string = $this->escapeString($string);
        $dquotes = substr_count('"', $string);
        if ($dquotes % 2 != 0)
            $string = $this->escapeString($string);
        return $string;
    }

        // conventional method to ensure the 'id' is only an id
        if (isset($_REQUEST['id'])) {
            if (isset($_GET['id']))
                $_GET['id'] = intval($_GET['id']);
            if (isset($_POST['id']))
                $_POST['id'] = intval($_POST['id']);

        // do the same for the other id's
        foreach ($_REQUEST as $key=>$var) {
            if (is_string($var) && strrpos($key,'_id',-3) !== false) {
                if (isset($_GET[$key]))
                    $_GET[$key] = intval($_GET[$key]);
                if (isset($_POST[$key]))
                    $_POST[$key] = intval($_POST[$key]);

                $_REQUEST[$key] = intval($_REQUEST[$key]);
            }
        }
            } elseif (count($this->url_parts) == 1 || $db->selectObject('section', "sef_name='" . substr($this->sefPath,1) . "'") != null) {
                if (($db->selectObject('section', "sef_name='" . $this->url_parts[0] . "'") != null) && (in_array(array('printerfriendly','exportaspdf','ajax_action'), $this->url_parts))) {
        return expString::escape(expString::sanitize($url));
        $this->sefPath = expString::escape(expString::sanitize($sefPath[0]));
//            if(0) {
//                // remove whitespaces and tags
////            $data = strip_tags(trim($data));
//                // remove whitespaces and script tags
//                $data = self::strip_tags_content(trim($data), '<script>', true);
////            $data = self::strip_tags_content(trim($data), '<iframe>', true);
//
//                // apply stripslashes if magic_quotes_gpc is enabled
//                if (get_magic_quotes_gpc()) {
//                    $data = stripslashes($data);
//                }
//
//                $data = self::escape($data);
//
//                // re-escape newlines
//                $data = str_replace(array('\r', '\n'), array("\r", "\n"), $data);
//            }
//                $tmp =1;  //FIXME we've already sanitized at this point
            if (!$user->isAdmin())
                expString::sanitize($_GET);