Opened 3 years ago
Last modified 2 months ago
#879 reopened дефект
Уязвимость для SQL injection
Reported by: | san | Owned by: | Denis_N |
---|---|---|---|
Priority: | major | Component: | БД изделий АДС |
Keywords: | Cc: |
Description
Возможно в базе остались уязвимости для SQL injection.
Нужно проинспектировать код и устранить уязвимости
Change History (13)
comment:1 by , 2 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 2 years ago
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");
comment:4 by , 2 years ago
Во-первых, предложение #870 по-моему никак не связано с мультизапросами. Предложение #870 решается (в общем случае) четырьмя запросами:
О, спасибо за уточнение, на слово транзакция я не обратил внимание.
утверждения о том, что prepared statements не могут использовать мультизапросы
Вот тут:
Использование множества выражений в подготавливаемом запросе не поддерживается.
https://www.php.net/manual/ru/mysqli.quickstart.multiple-statement.php
И, я так понимаю, ничего не помешает внутри транзакции использовать подготовленные запросы и мои опасения, что это помешает #870, не оправдаются.
В таком случае предлагаю использовать в интерфейсе базы только подготовленные запросы, для обеспечения гарантированной защиты от инъекций.
comment:6 by , 2 years ago
Resolution: | → готово |
---|---|
Status: | assigned → closed |
comment:7 by , 2 years ago
Resolution: | готово |
---|---|
Status: | closed → reopened |
Денис вспомнил, что на главной странице подготавливаемые запросы ещё не реализованы.
follow-up: 13 comment:11 by , 3 months ago
Replying to Denis_N:
Также для вставляемой строки для сортировки (order by) сделал отдельную функцию checkStr()
А это так и задумано, что строка
$str2 = substr($checkStr, $substrParam1, $substrParam2);
выполняется всегда, а переменные $substrParam1 и $substrParam2 определены только при некоторых услових?
И еще, простите за любопытство, но раз уж все равно пишу комментарий, спрошу - а зачем в запросе "distinct"? Разве может быть несколько столбцов с одним и тем же именем?
comment:13 by , 3 months ago
Привет, Алесей =)
Спасибо, согласен, не правильно сделал, что "переменные $substrParam1 и $substrParam2 определены только при некоторых условиях". Исправил это в https://trac.adc-line.ru/mc-04/changeset/368/base
А про distinct в запросе - при выводе (без distinct) дублируются столбцы UID, comment. Они есть и в products, и в history
Для того чтобы навсегда забыть про возможные инъекции, я хотел предложить использовать подготовленные запросы.
Однако в таком случае невозможно будет использовать множественные запросы, что, как я понял, подразумевается в #870.
С другой, стороны множественные вопросы сами по себе повышают риск инъекций, по сравнению с обычными:
Хотел бы услышать ещё мнения.