Comments 148
Так не лучше?
$sql = "SELECT user FROM userslist WHERE userid='".mysql_real_escape_string($_GET['uid'])."'";
UFO just landed and posted this here
да, еще бы все так писали…
UFO just landed and posted this here
Биндинг к MySQL API практически прямой. На Си почти те же самые функции.
Уже объявлено depricated.
Уже объявлено depricated.
Простите, а где написано что она deprecated? И чем ее предлагается заменять, кроме ORM?
Сорри, не depricated вроде, а not recommended (был уверен в первом). На замену mysqli или PDO_MySQL.
о как, спасибо! Где-то уже натыкался на mysqli, но решил тогда, что это вообще другая СУБД.
Как по мне, так полная ерунда. С таким же успехом можно признать не пригодными еще достаточное количество полезных функций. Составляя запрос нужно думать как человек, который планирует SQL-инъекцию. Сама по себе mysql_query ни хорошая и не плохая — это просто инструмент, а как им пользоваться — личное дело каждого.
UFO just landed and posted this here
Мне вообще непонятно, чем думали авторы PHP, когда добавляли туда mysql_query.
А почему нет? Их вины в небезопасном использовании функции разработчиками помоему нет.
UFO just landed and posted this here
А почему MySQL делали такую функцию? Это как бы язык программирования, а не игрушка для тех кто младше 5 лет.
Во всех книгах по PHP написано, не доверять данным которые пришли из web.
Во всех книгах по PHP написано, не доверять данным которые пришли из web.
Лучше так:
$result = $users->get( $params->uid );
нет, зачем для числа вызывать дорогую функцию mysql_real_escape_string, если приведение типов дешевле?
Только реальное экранирование, для реальных пацанов.
А вообще среди моих знакомых это частая практика — использовать громоздкие решения, там где есть изящные.
А вообще среди моих знакомых это частая практика — использовать громоздкие решения, там где есть изящные.
Да это ж любимая фишка гавнокодеров, они прям мечтают найти одну функцию которая от всех проблем помогает. Они бывает еще strip_tags и htmlentities добавляют для чисел, потому что где вычитали, что это защищает от XSS :)
Плюс к комменту выше: ещё не нужный оверхид с формированием и передачей кавычек (пускай пара байт, но всё же) и приведением строки в число уже в СУБД (тоже мелочь, но всё же). То есть ни одного плюса у этого варианта по сравнению с приведением типа в скрипте нет. Зачем он?
Между прочим, приведение типа — не мелочь. У меня как-то раз один запрос работал на два порядка медленнее из-за сравнения числа со строкой, в то время как можно было сравнивать напрямую два числа.
Вероятно потому, что из-за кастования субд не смогла использовать индексы. В то время как во время генерации запроса операция приведения типа ничего не стоит.
О как. Не зря значит привожу тип в php, а не в мускуле. Хотя чисто из эстетических соображений.
Ну вообще говоря у mysql были (может сейчас уже исправлено) ситуации, когда по непонятным причинам он в выражениях вроде
WHERE int_field = 'int_value'
приводил к числовому типу не правую, а к строковому левую часть. И получали фуллскан. Сам лично видел такое, да и в интернетах статьи проскакивали о.
WHERE int_field = 'int_value'
приводил к числовому типу не правую, а к строковому левую часть. И получали фуллскан. Сам лично видел такое, да и в интернетах статьи проскакивали о.
Заведомо числовые данные, пришедшие от пользователя, проверяю с помощью RegExp.
Текстовые данные прогоняю через функцию
mysql_real_escape_string
Ведь правильно же?
Текстовые данные прогоняю через функцию
mysql_real_escape_string
Ведь правильно же?
Нет.
Зачем регулярки и почему не использовать intval() или приведение типов?
Да и вообще лично я делаю запросы с использованием placeholder's в любых ЯП где сталкиваюсь с SQL базами данных.
Да и вообще лично я делаю запросы с использованием placeholder's в любых ЯП где сталкиваюсь с SQL базами данных.
placeholder — это очень правильно.
Плейсхолдер плейсхолдеру рознь. Наверное, вы всё же имеете в виду prepared statements.
Хммм… ну да, а можно поподробнее о разнице?
sprintf — тоже placeholder…
Просто placeholder — это общее понятие, которое просто обозначает место в шаблоне, в которое будет подставлено значение.
Например, в C#:
«Значение» будет подставлено вместо placeholder'а {0}. Но это не значит, что это безопасно с точки зрения SQL. То же самое и в PHP:
%s — плейсхолдер. Но к безопасности такие плейсхолдеры отношения не имеют.
В то же время, одно из предназначений prepared statements — как раз безопасность. Они тоже подставляют значения в плейсхолдеры, но делают это не напрямую (в виде строки), а в виде отдельных параметров-значений, на уровне базы данных.
Например, в C#:
String.Format("SELECT * FROM table WHERE column = {0}", "значение");«Значение» будет подставлено вместо placeholder'а {0}. Но это не значит, что это безопасно с точки зрения SQL. То же самое и в PHP:
sprintf("SELECT * FROM table WHERE column = %s", "значение");%s — плейсхолдер. Но к безопасности такие плейсхолдеры отношения не имеют.
В то же время, одно из предназначений prepared statements — как раз безопасность. Они тоже подставляют значения в плейсхолдеры, но делают это не напрямую (в виде строки), а в виде отдельных параметров-значений, на уровне базы данных.
А зачем выполнять запрос, если данные не корректны? intval _всегда_ вернет число, даже если пришла строка.
if( is_int($id) )
{
«select * from users where id=». $id;
}
И не нужноничего экранировать. И лишнего запроса не будет.
if( is_int($id) )
{
«select * from users where id=». $id;
}
И не нужноничего экранировать. И лишнего запроса не будет.
is_int($_GET['id']) — всегда false
Для начала сравните мой кусок со своим, а потом что-то говорите =)
Специально для вас можно использовать is_numeric, подойдет для обоих ситуаций.
Специально для вас можно использовать is_numeric, подойдет для обоих ситуаций.
А мускул поймёт id = 0xff?
echo is_numeric('0xff'); // 1
Поймёт, но очень по-своему :-) dev.mysql.com/doc/refman/5.5/en/hexadecimal-literals.html
Вообще я вел к тому что интвал всегда возвращает число, даже если строка прийдет, и будет очень неприкольно, если в базе есть данные со значением 0, да и в любом случае не прикольно выполнять запрос, если он в принципе не нужен. Поэтому считаю что нужно сперва проверить число ли пришло, и затем, если да, выполнять уже запрос. Как-то так =) Ато лепят сразу преобразовав, а ошибку человеку увидеть не судьба.
Запрос вроде id=0 выполнится мгновенно, потому что 0 < MIN(id), так что мускул даже пытаться искать по индексу не станет.
Согласен по части «зачем делать лишнюю работу», но с точки зрения программиста — написание этого условия это как раз та самая лишняя работа :-)
Согласен по части «зачем делать лишнюю работу», но с точки зрения программиста — написание этого условия это как раз та самая лишняя работа :-)
стоит уточнить, что id может быть таки и отрицательным, особенно если не auto_increment :)
Технически — естественно :-) Практически — приведи пример, когда это имело бы смысл?
например когда мы говорим не про первичный ключ и вообще не про индекс.
Зависит от ситуации. Лично я на практике в Postresql использую отрицательные значения id для подчеркивания факта наличия «системных» записей. Это как номера TCP портов до 1024 или же uid-ы в линях до 1000-чи.
Просто к основному sequence инкремент возрастающего вверх от нуля есть sequence для системных записей который инкрементится от нуля вниз.
В общем в итоге это не более чем принятое соглашение. Какого соглашение для практического использования приняли, такое и работает.
Просто к основному sequence инкремент возрастающего вверх от нуля есть sequence для системных записей который инкрементится от нуля вниз.
В общем в итоге это не более чем принятое соглашение. Какого соглашение для практического использования приняли, такое и работает.
Регуляркой делать.
Прекрасно выбирает строки с айди 19 =)
SELECT *
FROM transactions
WHERE user_ = 0x13
Вы ответили на комментарий, где автор рекомендует использовать intval. Написали, что intval вернет всегда число, поэтому надо делать is_int. И откуда тогда в контексте возьмется $id? Я вижу два варианта: $id = $_GET['id'] и $id = intval($_GET['id']). В обоих случаях смысл вашего ответа непонятен.
Функцию is_numeric я знаю. И если вы хотели написать ее вместо is_int, то достаточно было бы ответить, что вы перепутали функции.
Функцию is_numeric я знаю. И если вы хотели написать ее вместо is_int, то достаточно было бы ответить, что вы перепутали функции.
Да, я так и понял что написал непонятный коментарий =)
habrahabr.ru/post/142599/#comment_4775262 Тут я обьяснил подробно.
habrahabr.ru/post/142599/#comment_4775262 Тут я обьяснил подробно.
>Для защиты от этой шаблонной уязвимости, лучше всего использовать приведение типов.
ОК, а если в get-параметре передаётся не число, а строка?
ОК, а если в get-параметре передаётся не число, а строка?
тогда как в примере, только в обрамлении кавычками
Вы получите в приведенном значение 0, что не есть ошибка безопасности.
Я обычно использую следующий подход:
Я обычно использую следующий подход:
$id = (int)$_GET['id'];
А как обезопасить себя, если в get параметре строка и эта строка нужна пример для поиска по базе и не использовать placeholders?
addslashes поможет?
$sql = «SELECT * FROM `table` WHERE title LIKE '%$search%'»;
addslashes поможет?
$sql = «SELECT * FROM `table` WHERE title LIKE '%$search%'»;
Как-то так…
$search = base64_encode("%$search%");
$sql = "SELECT * FROM `table` WHERE title LIKE FROM_BASE64('$search')";
А для точного поиска можно использовать md5.
Жесть какая. Оверхед же.
Кто же спорит? Вот человек не может использовать подготовленные выражения, а такой способ позволит пускать строку как есть и при этом не бояться.
mysql_real_escape_string и кавычки чем хуже?
mysql_real_escape_string — изменяет строку.
base64_encode не изменяет что ли?
угу. вот только FROM_BASE64 в mysql появилась только в версии 5.6…
Для поиска я использую полнотекстовый поиск, но планирую перейти на Sphinx
Гляньте на ElasticSearch
Почему не использовать placeholders? Это к тому же наглядно:
$db = new mysqli(...);
$stmt = $db->prepare('update table_name set var1 = ?, var2 =? where var3 = ?');
$stmt->bind_param('sdd', $var1, $var2. $var3);
if ($stmt->execute())
{
…
}
При этом, в теории (ну не нужно мне было это проверять), подготовленный запрос можно использовать несколько раз вроде как без «перекомпиляции». А во вторых можно использовать именованные placeholders, в случае с PDO.
$db = new mysqli(...);
$stmt = $db->prepare('update table_name set var1 = ?, var2 =? where var3 = ?');
$stmt->bind_param('sdd', $var1, $var2. $var3);
if ($stmt->execute())
{
…
}
При этом, в теории (ну не нужно мне было это проверять), подготовленный запрос можно использовать несколько раз вроде как без «перекомпиляции». А во вторых можно использовать именованные placeholders, в случае с PDO.
Я имел ввиду, что это не годится для случаев, когда там и должна быть строка.
Например, так:
Впрочем, это я уже придираюсь, наверное.
Например, так:
$username = $_GET['username'];
// обеззараживаем $username
// ...
mysql_query("SELECT ALL FROM blablabla WHERE username = "{$username}");
Впрочем, это я уже придираюсь, наверное.
Если в таком запросе должна быть строка, то из-за отсутствия кавычек банально ничего не заработает и даже самый ленивый и неумелый программист пофиксит еще до инъекции.
Есть такая вещь как унаследованный код. И совершенно случайно работа с БД в нём в отдельный слой не выносится, как правило, хорошо если хоть какие-то слои есть.
Я о том только, что если ты даешь советы, — напиши основное.
ИМХО если мы тыкаемся в legacy код и видим в нем грязные mysql_ с невнятным поведением при некорректных аргументах — мы просто берем и заменяем эти вызовы на *ABSTR_DB_LAYER* (мы же новый код не по тем же принципам пишем, верно? у нас абстракции, модульность, программирование на уровне интерфейсов и вообще лучшие практики курят в сторонке).
Не всегда это просто.
Да. Не всегда.
У меня недавно был проект без архитектуры на инклудах, где магическая переменная магической погоняет. Это настолько классический случай лапши и плохого дизайна, что все сурсы можно запросто выкинуть на govnocode и получить +300 их рейтига.
Тогда я честно сказал заказчику, что если он хочет от меня что-то новое или правки старого функционала, это значит что мы будем удалять(выжигать железом) старый код и внедрять новый.
Понимаю, что не всегда дают деньги и время на это, но если нет времени на рефакторинг, значит с менеджментом что-то не так.
У меня недавно был проект без архитектуры на инклудах, где магическая переменная магической погоняет. Это настолько классический случай лапши и плохого дизайна, что все сурсы можно запросто выкинуть на govnocode и получить +300 их рейтига.
Тогда я честно сказал заказчику, что если он хочет от меня что-то новое или правки старого функционала, это значит что мы будем удалять(выжигать железом) старый код и внедрять новый.
Понимаю, что не всегда дают деньги и время на это, но если нет времени на рефакторинг, значит с менеджментом что-то не так.
Простите, а подход «не использовать вообще конкатенацию запросов, только передачу параметров» в PHP невозможен?
одно непонятно — зачем пускать к базе непроверенные запросы-то? :) Одного if жалко?
Переменная дешевле чем два intval, а if дешевле чем холостой запрос к базе.
$g_uid = intval($_GET['uid']);
if ($g_uid == $_GET['uid']) {
$sql = "SELECT user FROM userslist WHERE userid=".$g_uid;
}Переменная дешевле чем два intval, а if дешевле чем холостой запрос к базе.
Почему два intval'а то? Один. И у вас один. Только лишние if и переменная. Тем более параметров зачастую не один, а больше. Соответственно, вы что, будете в if делать проверку 5-10 переменных?
А почему бы и нет, если это касается безопасности.
Если превратить код в помойку ненужными проверками, то безопаность от этого только проиграет. Нет ничего страшного в выборке «WHERE id = 0», которая случается только в случае крайне редких махинаций. Да и если постоянно идут запроса «WHERE id = N» (N > 0 ), то «WHERE id = 0» явно не повесит сервер.
>Если превратить код в помойку ненужными проверками
Это зависит от уровня восприятия. Если для вас if и пустые запросы в порядке вещей, то для меня это не приемлемо.
Это зависит от уровня восприятия. Если для вас if и пустые запросы в порядке вещей, то для меня это не приемлемо.
Первый вариант:
Второй вариант:
Я обычно использую второй, потому что разницы по сути нет, а вот писать каждый день сотни лишних строк я не хочу (времени жалко).
if (!isset($_GET['id'])) Core::forward404();
$topicId = intval($_GET['id']);
if ($topicId < 1) Core::forward404();
$db = Core::getDatabase();
$topic = $db->selectRow('SELECT * FROM topics WHERE topic_id = ?d', $topicId);
if (!$topic) Core::forward404();
...
Второй вариант:
$topicId = isset($_GET['id']) ? intval($_GET['id']) : 0;
$db = Core::getDatabase();
$topic = $db->selectRow('SELECT * FROM topics WHERE topic_id = ?d', $topicId);
if (!$topic) Core::forward404();
...
Я обычно использую второй, потому что разницы по сути нет, а вот писать каждый день сотни лишних строк я не хочу (времени жалко).
Запрос, в котором не выполняется условие
intval($_GET['uid']) == $_GET['uid'], может быть вполне корректен семантически. Да и даже если нет, проще сделать холостой запрос, чем «пачкать» код проверками, если число холостых пренебрежимо мало.Можно несколько проще
if (is_numeric($_GET['uid']))
...
Вы ПХП-то знаете вообще? Суньте в _GET например такую строку: «9 AND UNION SELECT 1 FROM DUAL». Она чудесно пройдёт проверку.
К строке
К числу
$sql = "SELECT id FROM userslist WHERE username='" . mysql_real_escape_string($_GET['username']) . "'";
К числу
$sql = "SELECT user FROM userslist WHERE userid='" . intval($_GET['uid']) . "'";
Я обычно так пишу. Почти))
Не знаю, есть ли какой-то дополнительный смысл в наличии/отсутствии кавычек вокруг имен таблиц и столбцов, но мне так проще читать код
$sql = "SELECT `user` FROM `userslist` WHERE `userid`='" . intval($_GET['uid']) . "'";
Не знаю, есть ли какой-то дополнительный смысл в наличии/отсутствии кавычек вокруг имен таблиц и столбцов, но мне так проще читать код
В кавычках вокруг intval смысла нет, если userid число.
Я хотел бы услышать комментарий про другие кавычки)
В любом случае, предпочитаю ставить кавычки везде, в том числе вокруг intval
В любом случае, предпочитаю ставить кавычки везде, в том числе вокруг intval
Без их использования есть риск потерять совместимость с будущими версиями (My)SQL. Также иногда можно получить ошибку при попытке использовать зарезервированное слово и приходится оборачивать (если увидел ошибку). Потому правильным считаю ставить в запросах для единообразия, но зачастую лень, если специального требования нет. Если же речь идёт о чём-то универсальном, где имя таблицы/поля заранее неизвестно (например задаётся пользователем), то нужно ставить однозначно.
Комментарий на эту тему: FYI: escaping table/field names is only required if you're using reserved words. Dumping everything into backticks just makes the query that much harder to read.
Кстати, лучше всё-таки „(int)“, в ПХП вызов функции — дорогая операция.
Я использую библиотеку DbSimple от Котерова. На сколько мне известно, она используется и на этом движке, на котом хабр.
Аналогично. Использую допиленный DbSimple. Плюс писал свою версию с нуля для MSSQL, так как очень не хватало привычных методов при работе с этой СУБД.
Ссылка: dklab.ru/lib/DbSimple/ (много примеров)
$ids = array(1, 101, 303);
$db->select('SELECT name FROM tbl WHERE id IN (?a)', $ids);
...
$name = $_GET['name'];
$db->selectRow('SELECT * FROM users WHERE name = ?', $name);
...
$row = array('name' => 'Hint', 'age' => 25);
$db->query('INSERT INTO users SET ?a', $row);
Выглядит красиво, но как производится сама проверка?
В комментарии я привел далеко не все плюшки.
Все подстановки с "?" экранируются как строки. Все элементы массивов при подстановке "?a" экранируются как строки, а ключи массивов при UPDATE и INSERT как идентификаторы (`name`).
Для подстановок с "?d" выполняется intval:
Все подстановки с "?" экранируются как строки. Все элементы массивов при подстановке "?a" экранируются как строки, а ключи массивов при UPDATE и INSERT как идентификаторы (`name`).
Для подстановок с "?d" выполняется intval:
$topic = $db->selectRow('SELECT * FROM topics WHERE id = ?d', $_GET['id']);
Использую goDB, там всё тоже, только вместо множества методов указывается формат в котором представлять результат:
$db->query('SELECT * FROM users WHERE name=?', array($name))->row()
А разв Хабр на php? Я почему-то думал, что он на asp.
Обычно использую:
+ ессно автоэкранирование кавычек в строках
".select .. from ... where strcmp(field,'".$mustBeString."')=0 or id=".(int)$mustBeInt.";"+ ессно автоэкранирование кавычек в строках
Мой сайт хакали именно через такую SQL инъекцию. Век живи, век учись.
Если заведомо число передается, всегда делают some sql query where id=".(int)$value."… а вообще стараюсь чтобы всегда только числа передавались, а уж если строка, то кучу перепроверок + экранирование.
Тут народ intval предлагает, чем то лучше или хуже (а то вдруг 6 лет зря (int) пихаю)?
Тут народ intval предлагает, чем то лучше или хуже (а то вдруг 6 лет зря (int) пихаю)?
Только параметризованные запросы. Никаких конкатенаций. В PHP есть параметризованные запросы? Простите давно не писал на нем, не помню.
А под фреимворки регулярки не составляли?)
Когда нарочно обходят использование preapared statements конкатенацией.
Когда нарочно обходят использование preapared statements конкатенацией.
Не составлял, сложно и муторно. Есть средства статического анализа кода, они позволяют выполнять эту работу более комплексно и не привязаны к конкретным конструкциями.
Средства статистического анализа кода, написанного на PHP-фреймворках?
Не подскажите какой-нибудь? Буду благодарен.
Статистический анализ кода так или иначе привязан к конкретным конструкциям.
Не подскажите какой-нибудь? Буду благодарен.
Статистический анализ кода так или иначе привязан к конкретным конструкциям.
у нас своя разработка статического анализатора. Статический анализ привязан скорее уже не к конструкциям, а к структуре кода, скорее всего вы про это и говорили. Поэтому такие вещи как RIPS не справляются с ООП. Но если знать, что хочется найти, то, скажем, выполнить задачу «поиск всех функций где аргумент попадает в SQL запрос без фильтрации» выполнить можно быстро и качественно.
А, понятно, у вас свой анализатор. Просто думал какой-то инструмент используете с уже собранными регулярками.
Действительно, теперь про анализ мы друг друга поняли.
С последним, да, у меня у меня именно так: наткнулся на уязвимую конструкцию и грепаешь весь код на подобные, действительно, быстро и качественно.
Действительно, теперь про анализ мы друг друга поняли.
С последним, да, у меня у меня именно так: наткнулся на уязвимую конструкцию и грепаешь весь код на подобные, действительно, быстро и качественно.
Ага, тоже так делал, пока не устал эти регулярки писать. Очень много получается веб-приложений и все такие разные, что пришлось изобретать какую-то утилиту под свои нужды. В целом, посмотрите на TXL (http://www.txl.ca/ www.txl.ca/examples/Grammars/PHP/README.txt)
только экранирование, только хардкор!
Sign up to leave a comment.
Самый частый шаблон SQL инъекций в РНР — бесполезное экранирование символов