Защита от SQL inj

Question

[zobov]

Привет всем.
Возник такой вопрос — является ли достаточной такая фильтрация от sql inj?
foreach($_REQUEST as $_ind => $_val) {
$_REQUEST[$_ind] = mysql_real_escape_string($_REQUEST[$_ind]);
}

foreach($_POST as $_ind => $_val) {
$_POST[$_ind] = mysql_real_escape_string($_POST[$_ind]);
}

foreach($_GET as $_ind => $_val) {
$_GET[$_ind] = mysql_real_escape_string($_GET[$_ind]);
}

foreach($_COOKIE as $_ind => $_val) {
$_COOKIE[$_ind] = mysql_real_escape_string($_COOKIE[$_ind]);
}


Этот кусок кода инклудится в самом начале каждого скрипта. PHP 5.3.8

Answer 1

[Melkij]

Поздравляю, вы изобрели Magic Quotes.

Да, этого достаточно. Разве ещё только $_FILES необработан.
Нет, это неудобно. Из-за чего от магических кавычек и отказались.

Comments

[Константин]

Вы серьёзно?!

[Eirenliel]

Используйте mysql_real_escape_string при подстановке переменной в каждом запросе.

Answer 2

[Константин]

Говнокод детектед?
Интересно, как будете сравнивать значения в PHP? Например Если Вам понадобится сравнить два стринга а один из них будет изменен уже через вашу функцию(mysql_real_escape_string).
Эта функция должна вызыватся над стрингами непосредственно перед вложением в базу данных.
И вообще, хватит использовать старый mysql! Используйте mysqli: cz.php.net/manual/en/mysqli.prepare.php! Познакомтесь с препаре стейтментами и ваш геморой уйдёт в прошлое.

Comments

[Константин]

*препаре стейтментами = prepared statement

Answer 3

[Elkaz]

Use PDO, Luke :)

Answer 4

[Vladson]

И опять мифическая «защита»…

Проснитесь люди, от «SQL-inj» надо не защищаться, это не угроза. Наличие SQL-дыр это простой баг. Хотите избежать, не допускайте ошибок в коде.

Каким путём вы передаёте данные в запрос дело ваше, можно даже с помощью bin2hex
SELECT * FROM `table`
WHERE `word`=0x4841434b454420425920564c4144534f4e;
(хотя есть способы и проще и логичнее)
Главное поймите что данные и то что SQL сервер воспримет за данные, это разные вещи.

Но перестаньте называть это защитой, это не защита, а просто «код без бага».
(Защита подразумевает угрозу которая существует даже в случае грамотного, как в случае с DDOS это вполне нормальные пакеты, только их много.)

Comments

[Vladson]

И да, то что в вопросе, это говнокод. Данные трогать нельзя вообще. Можно сделать копию данных и обозвать «данные для SQL запросов» но исходные данные (те что в переменных GET/POST итд, трогать нельзя!!! ибо если вначале скрипта вы кидаете их в базу, то нет никаких гарантий что в конце скрипта вам не понадобится отрпавлять их куда-либо ещё, например на мыло или на HTML-шаблонизатор а данные уже будут испорчены)

Answer 5

[Eirenliel]

Используйте mysql_real_escape_string при подстановке переменной в каждом запросе.

Извиняюсь, не умею отвечать на Хабре...

Answer 6

[mapron]

если есть пхп 5.3 (а не пхп4), то грех уже не использовать mysqli, и забыть про инъекции с плейсхолдерами как страшный сон. Плюс экономия на времени парсинга запроса как приятный бонус)
в зенде допустим:
$db->query('UPDATE users SET login_time=? WHERE id=?', array($time, $id));
выше уже говорили про PDO. но к сожалению он включен не по дефолту, а MYSQLI использовать можно вполне. Имитация magic quotes напомнила мне, если честно:

//Далее не безопасный код, но только так можно исправить ситуацию со старой версией.

if ($_GET) {
    foreach ($_GET AS $key => $value) {
        $$key = $value; //Делаю все переменные GET простыми, как это было с reg_glob
    }
}

Comments

[Константин]

Почему мне Вас за это: $$key = $value; хочется убить? А ТЕМ БОЛЕЕ $value идёт из $_GET! Вы-же видите, человек новичёк, а учите его такому небезопасному коду!!! Он же потом себе все зубы поломает!!!

Answer 7

[zobov]

понял, всем спасибо за советы! =)

Comments

[Константин]

Мужик, используй или PDP или MySQLi, иначе себе зубы выбьешь.

[zapimir]

Вот такие советы как раз вредны, лучше человеку объяснить, как правильно делать и почему, а не просто писать используй ЭТО и будет тебе счастье, а то человек к этому будет относиться, как к какой-то магии, и надеяться, что они все сделают сами, за него.

[Константин]

Опять-же не согласен. Я как программист, иду первым образом прочитаю об той или инной функции мануал.
Извини, но здесь прозвучало много разных мнений + внизу появился говнокод. Я лучше скажу как да что должно быть и магия будет работать, чем разжевывать всё подряд. Ведь если кто не поймёт, пожалуйста, пусть спрашивает, с удовольствием чем смогу помогу.

Answer 8

[edogs]

По поводу самой идеи выше уже все всё сказали сказали… хотя если в скриптах есть тонна г-но кода, где программер не озабачивался прослэшиванием переменных, то такой способ может быть быстрым спасением, до тех пор пока код не приведут в порядок.
Единственное что добавим — такой способ убьет массивы, если что-то где-то передается как input name=«a[2]», то кирдык данным после «прослэшивания».

Answer 9

[Виталий Желтяков]

Мой вариант:
//---------------- Процедура подключения к базе данных -----------------------//
function dbconnect() {
global $database;
if (!isset($database)) {
$database = mysql_connect("localhost", "**", "***");
mysql_select_db("xxx", $database);
}
}

//--------------------- Функция выполнения запросов --------------------------//
function sql_query($query) {
global $errors;
dbconnect();
$return = mysql_query($query);
$error = mysql_error();
if ($error=='') {
return $return;
}
else {
writelog('sql_error', date("y.m.d H:m:s")."\t".$error);
$errors .= $error;
return false;
};
}
//----------------------------------------------------------------------------//

//----------- Процедура проверки данных перед вставкой в запрос ---------------//
function checkfield($request) {
$request = trim($request);
if (isset($request)) {
// Если не число, то экранируем ковычки
if (is_numeric($request)) { return $request; }
else { dbconnect(); return mysql_real_escape_string($request); }
}
else { return ''; }
}
//----------------------------------------------------------------------------//

Comments

[Константин]

Мне не нравится ваш код :-).
1. global
2. простой MySQL
3. нет prepared statment (но это не существенно)
4. нон ООП подход (но это не существенно)
5. ПРИ КАЖДОМ СКВЛ запросе вы вызываете лишьную функцию. (но это не существенно)
6. Записываете ошибки не в поле а в одну строку (но это не существенно)
7. function checkfield($request) {… $request = trim($request); if (isset($request))… = всегда правда
8. ЗАЧЕМ ВОТ ЭТО: «else { dbconnect(); return mysql_real_escape_string($request); }» О_о
Этот подход был актуален этаких 5 лет тому назад и то код с ошибками.
Такое ощущение, что у вас всё время проподает связь с базой данных или вы её где-то переписываете.

[Виталий Желтяков]

Разжую по порядку:

1. «global» — Что лучше 1 глобальная переменная или ненужное подключение к серверу? По мне лучше я потеряю несколько байт памяти, чем до 0.003 сек. на запрос.
2. «простой MySQL» — для простых задач, он идеально подходит. Или Вам нужны какие-то особенности других СУБД?
3. «нет prepared statment» — это хорошо на однотипных запросах. Но кто сказал, что у нас однотипные запросы?
4. «нон ООП подход» — А зачем здесь ООП? Чтобы ресурсов больше съесть? PHP — это не С, в нём эффективнее использовать функциональное программирование.
5. «ПРИ КАЖДОМ СКВЛ запросе вы вызываете лишьную функцию. (но это не существенно)» — Во-первых, на это ресурсов практически не тратиться. Во-вторых, это не все функции проверки данных.
6. «Записываете ошибки не в поле а в одну строку (но это не существенно)» — Здесь все ошибки пишутся в логи построчно. А строка при необходимости отсылается на клиент.
7. «function checkfield($request) {… $request = trim($request); if (isset($request))… = всегда правда» Вы забываете случай пустой строки.
8. «ЗАЧЕМ ВОТ ЭТО: «else { dbconnect(); return mysql_real_escape_string($request); }» О_о» — Числовые значения экранировать не надо. А для вызова функции mysql_real_escape_string нужно подключение к базе.

«Такое ощущение, что у вас всё время проподает связь с базой данных или вы её где-то переписываете»
— Вы плохо разобрались в коде. Код заточен на то, чтобы не создавать лишних телодвижений (подключение к СУБД, экранирование), если это не нужно.

[Константин]

Я в ахе. Без коментариев. Мужик, давай вечерком сядем на ICQ, или по хабрапочте, я тебе обясню почему ты не прав?