CVE-2016-9283 (PHP, published 2016-11-11T22:59Z)
Description: value: SQL Injection in framework/core/subsystems/expRouter.php in Exponent CMS v2.4.0 allows remote attackers to read database information via address/addContentToSearch/id/ and a trailing string, related to a "sef URL" issue.
Severity: MEDIUM (CVSS v3 base score: 7.5)
Weakness: CWE-89 \u2014 Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection')
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.
Repository: https://github.com/exponentcms/exponent-cms
Files changed:
framework/core/subsystems/expRouter.php
Commit message: fix sql injection security vulnerability which failed to account for sef urls adequately; reported by many, many users


Vulnerable code:
    public  $url_style = '';
    public  $sefPath = null;
        // strip out possible xss exploits via url
                $_GET['id'] = intval($_GET['id']);
                $_POST['id'] = intval($_POST['id']);

            $_REQUEST['id'] = intval($_REQUEST['id']);
                    $_GET[$key] = intval($_GET[$key]);
                    $_POST[$key] = intval($_POST[$key]);

                $_REQUEST[$key] = intval($_REQUEST[$key]);
        // start splitting the URL into it's different parts
        $this->splitURL();
        // edebug($this,1);

                        $_POST['controller'] = $module;
                        $_REQUEST['controller'] = $module;
            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);
            } 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))) {
            $this->url_style = 'sef';
//fixme and we may also want to log any redirects taken??
                $this->params = $this->convertPartsToParams();
                $return_params['url_parts']['id'] = $this->url_parts[2];
        // Set the module or controller - this how the actual routing happens
        $_REQUEST[$requestType] = $return_params['controller']; //url_parts[0];
        $_GET[$requestType] = $return_params['controller'];
        $_POST[$requestType] = $return_params['controller'];
        $_REQUEST['action'] = $action;
        $_GET['action'] = $action;
        $_POST['action'] = $action;
        // pass off the name<=>value pairs
            $save_value = expString::sanitize($value);
            $_REQUEST[$key] = $save_value;
            $_GET[$key] = $save_value;
            $url .= substr(PATH_RELATIVE,0,-1).$this->sefPath;
            if (isset($_REQUEST['PHPSESSID'])) unset($_REQUEST['PHPSESSID']);
        if (substr($this->sefPath,-1) == "/") $this->sefPath = substr($this->sefPath,0,-1);

Fixed code:
    private $url_style = '';
    private $sefPath = null;
        // start splitting the URL into it's different parts
        $this->splitURL();
        // edebug($this,1);

        // strip out possible xss exploits via old school url
        //fixme only old school url and forms have these variables here
            $_REQUEST['id'] = intval($_REQUEST['id']);
                $_GET['id'] = $_REQUEST['id'];
                $_POST['id'] = $_REQUEST['id'];
                $_REQUEST[$key] = intval($_REQUEST[$key]);
                    $_GET[$key] = $_REQUEST[$key];
                    $_POST[$key] = $_REQUEST[$key];
            }
            if ($key == 'src') {
                $_REQUEST[$key] = preg_replace("/[^A-Za-z0-9@-]/", '', $_REQUEST[$key]);
                if (isset($_GET[$key]))
                    $_GET[$key] = $_REQUEST[$key];
                if (isset($_POST[$key]))
                    $_POST[$key] = $_REQUEST[$key];
                        $_POST['controller'] = $_REQUEST['controller'] = $module;
    /**
     * Assign url_type & url_style
     */
            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);
            else
                $this->url_parts[0] = expString::escape($this->url_parts[0]);
            } 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))) {
            $this->url_style = 'sef';  // even if it's old school, they all come in the same
                //note 'query' doesn't need $params
    /**
     * Set up for page request, but check store category/product also
     *
     * @return bool
     */
//fixme and we may also want to log any redirects taken from that table??
                $this->params = $this->convertPartsToParams(); // update params to new re-mapped url_parts
                //fixme do we need to re-sanitize them???
    /**
     * Check and set up for an action request
     *
     * @return bool
     */
                $return_params['url_parts']['id'] = intval($this->url_parts[2]);
        // Set the controller - this how the actual routing happens
        $_REQUEST[$requestType] = $_GET[$requestType] = $_POST[$requestType] = $return_params['controller'];
        $_REQUEST['action'] = $_GET['action'] = $_POST['action'] = $action;
        // pass off the name<=>value pairs for old school url
            $_REQUEST[$key] = $_GET[$key] = expString::sanitize($value);
            if (count($this->params) > 2) {
                $url .= substr(PATH_RELATIVE,0,-1).'/'.$this->params['controller'].'/'.$this->params['action'];
                foreach ($this->params as $key=>$value) {
                    if ($key != 'controller' && $key != 'action') {
                        $url .= '/' . $key . '/' . $value;
                    }
                }
            } else {
                $url .= substr(PATH_RELATIVE,0,-1).$this->sefPath;  //fixme do we need to clean this up?
            }
    /**
     * Convert url_parts() or $_REQUEST to $params
     *
     * @return array|string
     */
            if (isset($_REQUEST['PHPSESSID']))
                unset($_REQUEST['PHPSESSID']);
        // conventional method to ensure the 'id' is only an id
        if (isset($params['id'])) {
            $params['id'] = intval($params['id']);
        }
        // do the same for the other id's
        foreach ($params as $key=>$var) {
            if (is_string($var) && strlen($key) >= 3 && strrpos($key,'_id',-3) !== false) {
                $params[$key] = intval($params[$key]);
            }
            if ($key == 'src') {
                $params[$key] = preg_replace("/[^A-Za-z0-9@-]/", '', $params[$key]);
            }
        }
    /**
     * Attempt to locate a page by name or id
     *
     * @param $url_name
     * @return null|object|void
     */
    /**
     * Get the SEF URL from the server
     * if we got an old school url, it will only contain the 'index.php'
     */
        if (substr($this->sefPath,-1) == "/")
            $this->sefPath = substr($this->sefPath,0,-1);  //fixme isn't this redundant from above?