Уязвимость для SQL injection

[san]

Возможно в базе остались уязвимости для SQL injection.
Нужно проинспектировать код и устранить уязвимости

[san]

Для того чтобы навсегда забыть про возможные инъекции, я хотел предложить использовать ​подготовленные запросы.
Однако в таком случае невозможно будет использовать ​множественные запросы, что, как я понял, подразумевается в #870.
С другой, стороны множественные вопросы сами по себе повышают риск инъекций, по сравнению с обычными:

Рассмотрение аспектов безопасности
Функции API mysqli::query() и mysqli::real_query() во время работы не устанавливают на сервере специальный флаг, необходимый для выполнения мультизапросов. Отдельная API-функция для мультизапросов позволяет снизить вероятность случайных SQL-инъекций. Злоумышленник может попытаться добавить в конец запроса выражения, вроде ; DROP DATABASE mysql или ; SELECT SLEEP(999). Если ему это удастся, но не будет использоваться функция mysqli::multi_query, сервер не выполнит внедрённое и опасное SQL-выражение.

Хотел бы услышать ещё мнения.

[alx]

Replying to san:

Однако в таком случае невозможно будет использовать ​множественные запросы, что, как я понял, подразумевается в #870.
Хотел бы услышать ещё мнения.

Во-первых, предложение #870 по-моему никак не связано с мультизапросами. Предложение #870 решается (в общем случае) четырьмя запросами:

START TRANSACTION;
UPDATE `peoducts` SET ....;
INSERT INTO `history` .....;
COMMIT;

и ничто не мешает выполнять их отдельными вызовами API.

Во-вторых я нигде не видел утверждения о том, что prepared statements не могут использовать мультизапросы (хотя не встречал и утверждений обратного), типа таких:

$mysqli->prepare("UPDATE peoducts SET a=?, b=?; INSERT INTO history(a, b) VALUES(?, ?);")->bind_param("isis", 123, "abc", 456, "def");

[san]

Во-первых, предложение #870 по-моему никак не связано с мультизапросами. Предложение #870 решается (в общем случае) четырьмя запросами:

О, спасибо за уточнение, на слово транзакция я не обратил внимание.

утверждения о том, что prepared statements не могут использовать мультизапросы

Вот тут:

Использование множества выражений в подготавливаемом запросе не поддерживается.

​https://www.php.net/manual/ru/mysqli.quickstart.multiple-statement.php

И, я так понимаю, ничего не помешает внутри транзакции использовать подготовленные запросы и мои опасения, что это помешает #870, не оправдаются.
В таком случае предлагаю использовать в интерфейсе базы только подготовленные запросы, для обеспечения гарантированной защиты от инъекций.

[Denis_N]

Сделано и закончено в r99/base

[san]

Денис вспомнил, что на главной странице подготавливаемые запросы ещё не реализованы.

[san]

Milestone deleted

[Denis_N]

In 365/base:

Улучшение: Главная страница переведена на подготавливаемые запросы в тех местах, где есть возможность изменить данные подставляемые в текст запроса
See #879

[Denis_N]

In 367/base:

Полностью перевел главную на подготавливаемые запросы. Также для вставляемой строки для сортировки (order by) сделал отдельную функцию checkStr() для проверки отсутствия в ней sql-иньекции

See #879

[alx]

Replying to Denis_N:

Также для вставляемой строки для сортировки (order by) сделал отдельную функцию checkStr()

А это так и задумано, что строка

    $str2 = substr($checkStr, $substrParam1, $substrParam2);

выполняется всегда, а переменные $substrParam1 и $substrParam2 определены только при некоторых услових?

И еще, простите за любопытство, но раз уж все равно пишу комментарий, спрошу - а зачем в запросе "distinct"? Разве может быть несколько столбцов с одним и тем же именем?

[Denis_N]

In 368/base:

Исправлен баг: substr выполняется всегда, а переменные $substrParam1 и $substrParam2 определены только при некоторых условиях
See #879

[Denis_N]

Привет, Алесей =)
Спасибо, согласен, не правильно сделал, что "переменные $substrParam1 и $substrParam2 определены только при некоторых условиях". Исправил это в https://trac.adc-line.ru/mc-04/changeset/368/base

А про distinct в запросе - при выводе (без distinct) дублируются столбцы UID, comment. Они есть и в products, и в history

[Denis_N]

In 577/base:

Исправлен баг: Сделано устранение ручной подстановки значений в SQL-запросах.

В интерфейсе сканера запрос получения сводки по изделиям с тем же наименованием переведён на подготовленный запрос с параметрами.
В интерфейсе отгрузки формирование списка серийных номеров для выборки данных заказа переведено на параметризованный IN (?, ...) с привязкой значений через bind_param.

See #879

[Denis_N]

Проверено текущее состояние по тикету.

Ранее были выполнены основные правки:

• r99/base: запросы в интерфейсе “Заказы” переведены на подготовленные запросы и транзакции;
• r365/base r367/base: Главная страница переведена на подготовленные запросы;
• r368/base: исправлена ошибка в проверке строки сортировки;
• r371/base: добавлена дополнительная проверка POST[orderHide].

Дополнительно проверил активный код интерфейса базы. Найденные оставшиеся хрупкие места были исправлены:

• в интерфейсе сканера запрос получения сводки по изделиям с тем же наименованием переведён на подготовленный запрос;
• в интерфейсе отгрузки формирование списка серийных номеров для выборки данных заказа переведено на параметризованный IN (?, ...) с bind_param.

После повторной проверки прямой подстановки пользовательского ввода в SQL в активном коде не обнаружено. Оставшиеся динамические SQL-фрагменты либо статические, либо ограничены whitelist, либо используют prepared statements.

Закрываю как исправленное.