Comments 80
У меня всего два совета к начинающим программистам, которые решат эту и многие другие проблемы:
1) Используйте фреймворки
2) Не изобретайте велосипеды
В данном случае это лишило бы вас ценных знаний и опыта.
А т.к. программист начинающий — скорее всего он лишь учится и тренируется, не пишет «боевых» решений, вы же с самого начала желаете лишить его фантазии, опыта и весьма полезных знаний.
Хотя конечно когда
Часто ко мне обращаются клиенты, у которых установлены самописные CMS или модули, написанные начинающими веб-программистами, которые не понимают, что нужно для защиты данных и зачастую копируют функции фильтрации, не задумываясь о том как они работают и что именно нужно с ними делать.

это печально.
Фреймовики имеет смысл использовать, когда уже есть опыт программирования.
Читать код этих фреймворков хотя бы, чтобы не было подобного рода статей.
я бы сказал, когда сам сможешь написать тоже самое, что и во фреймворке, но зачем тратить время и наступать на многие грабли.
htmlspecailchars и mysql_escape_string вместе вызывают жуткое жжение в заднице.
Вот разве так сложно понять, что в базу надо писать данные, а экранировать(если это нужно) — в шаблоне?
Это не всегда верно. Иногда данных так много (пример — количество комментариев на Хабре к какой-нибудь популярной статье), что каждый раз производить такие манипуляции просто невыгодно. Как вы думаете — комментарии на Хабре хранятся в исходном виде и парсятся каждый раз при выводе?
В изначальном виде хранятся однозначно.

Однако, вероятно, есть кеш и для варианта после парсинга(т.е. оригинал + обработанное).
Потому что потом парсер может измениться и хорошо было бы все комменты через него заново прогнать
Ну я бы не трогал прошлые комменты, если парсер обновляю. Ведь тогда комменты вида «парсер лох» — станут неактуальными.
Всё зависит от задачи.

Я просто предложил вариант, когда хранение исходных комментариев as-is было бы удобным

ps: в подавляющем большинстве комменты вида «парсер лох» возникают по причине невнимательности пользователей (неверно написали разметку или ещё что), а не плохого парсера.

ps: да и лично в подавляющем большинстве случаев предпочитаю хранить пользовательские данные в том виде, в котором они пришли, форматируя при выводе
С точки зрения производительности, лучше форматировать на входе, и дублировать оригинал (если нужно).

С точки хрения архитектуры, всё очень спорно)
Производительность — кеширование, неважно где. Это не обсуждается даже.

С точки зрения приложения: разные клиенты требуют разной обработки. Завтра вы захотите издавать хабрахабр в ПДФ, для него форматирование комментариев может быть другим.
Жаль что у нас контекст — php, иначе обсудили бы производительность).

Отдельная прослойка отображения — само собой, тут я с вами вынужден согласиться.
Если что, я имел в виду, что кеширование — является основой оптимизации только для php. Для языков вида C, C++, Delphi; C#, Java — это не так.

Парадигмы этих языков отличны от php тем, что контекст исполнения разных запросов (если уж о вебе говорить) — единый, что «принуждает» по умолчанию всё оставлять в ОП, что в контексте php — является кешированием.
Лучше хранить оригинал в БД, а, если нужно, форматировать в кеш или при записи, или при первом запросе. Кеш может быть и в БД, в той же таблице, что оригинал, например, comment.body и comment.body_safe_html.

В данном случае практической разницы вроде никакой по сравнению с «форматировать на входе, и дублировать оригинал» — те же два столбца, но идеологически или, если угодно, архитектурно, очень отличается — храним оригинал в raw и дублируем его с целью повышения производительности заранее форматированным и/или санированным для какого-то конкретного формата вывода. Просто надо понимать, что comment.body — это собственно данные, а comment.body_safe_html — это кеш конкретного их представления.
> Это не всегда верно.
Как вы определяете границу когда верно, а когда нет? Ну скажем, если на сайте появляется по 2000 комментов в год, то какой вариант выбрать? А если 100 000 в год?

А если на сайте есть «топики» и «комменты», при этом, топиков пишется по 100 в год, а комментариев 99999999 — вы будете использовать разные алгоритмы?
Используйте функцию mysql_escape_string Ответ неверный. правильный ответ — Используйте функцию mysql_real_escape_string.

if (count($checkbox)) // Проверяем, является ли переменная массивом

Так логичнее, не?
if (is_array($checkbox)) // Проверяем, является ли переменная массивом
mysql_real_escape_string не всегда подходит.
Функция идентична mysql_real_escape_string(), исключая то, что mysql_real_escape_string() принимает параметром ещё и указатель на соединение и экранирует в зависимости от кодировки. mysql_escape_string() не делает этого и результат работы не зависит от кодировки, в который вы работаете с БД.
Тем не менее не все хостеры бегут и обновляют версии ПО, до сих пор есть функции, которые ещё год или более были признаны как устаревшими и их ещё используют довольно успешно.
А что будет когда хостер внезапно обновит версию ПО?
Мне кажется не очень надежным пользоваться возможностями, которые могут исчезнуть в любой момент.
Дело не в том, что вы сидите на хрупкой ветке, способной обломится в любой момент. Нет!

Дело в том, что умные дядьки проанализировали ситуацию, помозговали и поняли, что функция mysql_escape_string не безопасна и рекомендовали её не использовать. Если бы не обратная совместимость, то они выпилили бы её сразу. Однако пока решили ограничится только пометкой «Depricated» и записью в мануале.

Причём тут динозавро-хостеры? Благодаря им вы можете ещё пару лет не переписывать свои старые наработки. Но зачем её использовать в новых библиотеках? И тем более, зачем она в статье на хабре, которую будут копипастить новички?
if (count($checkbox)) // Проверяем, является ли переменная массивом

Это вообще жесть, а не проверка. count() возвращает 1 почти для любых значений, в том числе для false и пустых строк. 0 будет возвращен только для пустого массива и null значения.
Зачем использовать голый mysql? Зачем хранить инклуд-php-файлы в докруте и тем более разрешать к ним http доступ? Почему секретку хранить надо в аргументах, а не в куке (как яндекс.бар сливал такие урлы, помните?) Почему надо выводить ошибки в браузер, когда для этого есть error_log, set_error_handler и tail -f? Пережитки шард-хостингов без доступа к логам? Это скорее частный случай, требующий особого подхода.
Поэтому пост и называется "… Частые ошибки". В идеале, конечно, лучше пользоваться фреймовиками (хотя мне проще самому написать скрипт), где уже всё готово и проверенно временем.
Да, я этих ошибок и сам насмотрелся. Ваша статья подойдёт для программистов (новичков) которым на плечи упал чей-то проект написанный вот в таком вот стиле. Но здесь ошибочен сам подход. Главное, чтобы никто из начинающих бойцов не воспринял этот материал как руководство к действию при старте.
я подумал что в первый раз вы ошидлись, но, видимо, это у вас ходовой термин.
Symfony, Code Igniter, Kohana — это framework, a не frameovik.
>Почему надо выводить ошибки в браузер, когда для этого есть error_log, set_error_handler и tail -f?

Помимо отладки (которую действительно можно улучшить так), нужно же как-то объяснить нестандартную ситуацию пользователю. Сказать, что тут нет контента, или сослаться на внутреннюю ошибку сервера (не вскрывая ошибки), или ещё что-нибудь. Лог-файлами тут не обойтись, увы.
Разве не для этого предназначены статусы http (403, 404, 500, 503, и т.д.)?
У автора явно весьма мало опыта в веб-программировании. Во-первых, путаница с понятиями «фильтрация» и «экранирование». Во-вторых, множество антисоветов (почти в каждом пункте, на самом деле).

Мой совет в вопросе безопасности — один: если вы хотите в явном виде написать в коде бизнес-логики вызов функции экранирования/фильтрации, вы уже дорускаете серьезнейшую архитектурную ошибку. Поэтому перепишите код так, чтобы экранирование в явном виде вообще нигде применять не нужно было бы (кроме сАмого низкоуровневого, самого последнего, библиотечного слоя, который очень мал и пишется 1 раз, и в котором злосчастная функция экранирования упоминается всего незколько раз, независимо от размера проекта). Только так можно защититься от инъекций (это касается, кстати, и xss тоже).

Иными словами, число вызовов функций фильтрации/экранирования должно быть константным и не зависеть от размера проекта (если вдруг имеет место пропорциональность или другая зависимость — это плохо).
Вообще конечно писать свои фильтрации и валидаторы стоит только ради того, чтобы потом использовать готовые уже решения и библиотеки и радоваться что за вас уже все продумали. Поэтому мой совет: НЕ ИСПОЛЬЗУЙТЕ НА ПРОДАКШЕНЕ СВОИ БИБЛИОТЕКИ
Прочитал все ошибки…

А не проще было написать:
1. Юзайте PDO
2. Юзайте filter_var / filter_input / etc.
3. Не изобретайте велосипеды
1. Юзайте PDO


PDO никак не человека, писавшего
$q = 'SELECT * FROM table where id='.$_GET['id'];


Так что совет — используйте подготовленные выражения. А еще лучше популярные абстракции над уровнем даннных — это позволит избежать возможных проблем при миграции, а также снимет часть типовых задач.

filter_var

О встроенный валидатор email в php я уже однажды обжегся. После этого везде использую валидатор yii(выдранный или вместе с фреймворком — в завсимости от ситуации).

Автору поста.
В файле index.php (или в любом другом главном файле) напишите такую строчку перед подключением других php файлов


Это действительно совет? Если да, то поясните при каком подходе к разработке приложения проблемы с инклудами нужно решать именно таким образом, пожалуйста.
Божечки, когда же люди перестанут пренебрегать новыми и правильными средствами языка, отмазываясь тем, что «хостеры не хотят обновлять версию PHP!»? Господа, 2012 год на дворе и можно купить VPS за 200-300 рублей, это если для себя. А если клиент такой прижимистый и для него есть разница между 150р в месяц и 200 — бегите от него!
Ну, например, Clodo или сраный FirstVDS даёт ISPManager. То ли бесплатно, то ли за какие-то копейки. Есть куча всяких панелек, которые не хуже хостерских.
Вариант, конечно, но какой-то неполноценный, по-моему. Ставить LAMP через консоль или панельку разницы особо не вижу, и там, и там конфиги дефолтные будут. А на хостингах — хочется надеяться, — тюнят их, причём не «настроили и забыли», а анализируя текущуюобстановку.
Ну, он рассчитывает, что они нормально настроены, особенно лимиты мускула по памяти и прочие буфера. Чтоб и память без дела не стояла и в своп не уходила (если он есть).
Зачем при подключении файла из $_GET[...] делать switch? o_O. Почему бы не воспользоваться:
if( in_array( $_GET[...], array( 'some', 'other' ), true ) )
{
    include ...;
}

мне кажется тут:
if (!$_POST['secret_key'] OR $_POST['secret_key'] != $secret_key)

Необходимо проверять строго $_POST['secret_key'] !== $secret_key.

Далее, ваша статья рассчитана на новичков, но тем не менее вы используете в качестве ключей такие слова как chislo. Возможно, новичкам _такое_ будет понятнее, но не стоит подавать им столь дурной пример.

По поводу «define ( 'READFILE', true );». Вчера начал изучать Kohana, и там используется такой же подход. С тех пор у меня в голове вертится 1 вопрос — не проще настроить .htaccess?
.htaccess, наверное, даже проще, но появляется гипотетическая вероятность его, например, потерять при переезде, или хостер может внезапно его зачем-то переписать — сталкивался с таким на шаред. А тут, вроде как, на уровне приложения контекст вызова проверяется.
В IDE шаблоны прописываются, поэтому меня лично вообще не напрягает.
А в Kohana и используется .htaccess. Вот прямо выдержка из рекомендуемого:
RewriteRule ^(?:application|modules|system)\b.* index.php/$0 [L]
Угу, я видел default.htaccess. Получается, все эти проверки просто на всякий случай, вроде внезапного переезда на неадекватных хостинг, установки nginx без настройки конфига и пр.?
Как всегда, в комментариях больше информации, чем в самой статье.
Тут уже на многое указали, но я все же добавлю.
Получается мы создаем запись в базе, которая совершенно бесполезна нам.

Это называется «не обеспечена целостность данных». Используйте ограничения на уровне скуля, гуглить «innodb foreign».

Куки лучше подписывать соленым хешем, при их отправке браузеру. Т.е. если нужно записать в куки значение
Cookie::getinstance()->set('key', 'value');

то в методе set к value, через разделитель, конкатенировать хеш этого значения, например md5('value'.md5(Cookie::$salt));, а сразу при обработке запроса проверять валидна ли подпись куки, и если нет — тереть такую куку, тогда для всего остального приложения ее просто не будет существовать.
Получился какой-то сборник bad practice. Не нужно вводить в заблуждение новичков (аргументации уже более чем достаточно в комментариях к топику выше).
foreach ($checkbox as $value)
{
	$value = intval($value);
	if ($value) $new_arr[] = $value;
}

В данном случае 0 пофильтруется, хотя является нормальным числовым значением.

$slovo = strip_tags($_GET['slovo']);
$slovo = htmlspecailchars($slovo);
$slovo = mysql_escape_string($slovo);

Зачем здесь strip_tags? htmlspecailchars достаточен, а strip_tags, к примеру, вырежет имейл из такой безвредной строки:
«Vasya Pupkin» <vasya@pupkin.ru>
К тому же заменять спецсимволы на сущности лучше на выводе, а не при записи.
В данном случае 0 пофильтруется, хотя является нормальным числовым значением.

Это лишь один из возможных вариантов проверки.
Ничего не мешает убрать её и поставить свою (на количество значений в массиве, на допустимость тех или иных значений).
Зачем здесь strip_tags?

Иногда мне не нужны html теги в переменной и лучше их сразу удалить, чем обходиться одним htmlspecailchars.
Кое-что поменял в посте, что всех не много раздражало (я про is_array).
В остальном, я не вижу ошибок в посте, только советы как тем или иным программерам удобнее/привычнее составлять условия.

Но пока я не увидел главного — что данный пост не помогает защитить свой скрипт от взлома.
Думаю сначала вам самому нужно разобраться в этой теме получше, а потом советовать что-то другим, особенно новичкам.
И в чём же мне нужно разобраться?) Хотите сказать, что этот пост никому не поможет?
Вопросы риторические, понятно, что смысл вашего поста был просто что-то написать, а не дать дельный совет.
> И в чём же мне нужно разобраться?) Хотите сказать, что этот пост никому не поможет?

Использование deprecated функций, дурной тон именования переменных… Продолжить? :)
Вот это:
>$slovo = strip_data($_GET['slovo']);
>$slovo = htmlspecialchars($slovo);
>$slovo = mysql_escape_string($slovo);
Отровенный пи*дец.

Во первых это мнимая безопасность, так как данные нужно обрабатывать от XSS перед выводом, а не перед вставкой.
Во вторых вы тупо портите исходные данные.

И дело даже не в именовании переменных, называйте их как хотите.
Во первых это мнимая безопасность, так как данные нужно обрабатывать от XSS перед выводом, а не перед вставкой.

О каком выводе, о какой вставке говорите?
Я показал пример фильтрации данных перед SQL запросом, нигде не написано, что это какой-то вывод.
Во вторых вы тупо портите исходные данные.

Я показал кусок из своего скрипта, где мне нужно было получить определенные данные, а не 1 в 1 как было передано через ПОСТ или ГЕТ…
И дело даже не в именовании переменных, называйте их как хотите.

Это ещё тут причём? Или надо было назвать «kojsub_get» ?)
По вашему названия переменных chislo и slovo нормальная практика? Т.е. вы считаете что за такое не надо бить руками и ногами? o_O. Или таки, не всё так плохо, и я просто не правильно вас понял?
Названия переменных я написал для примера, к чему эти придирки?)
Очень дурной пример. Очень.
по-моему у Вас еще маловато опыта что бы давать совета другим
$checkbox = $_POST['checkbox'];
$new_arr = array(); // Массив для сохранения отфильтрованных значений
if (is_array($checkbox)) // Проверяем, является ли переменная массивом
{
foreach ($checkbox as $value)
{
$value = intval($value);
if ($value) $new_arr[] = $value; // не допускаем нули
}
}

Можно заменить на
$new_arr = array_filter(array_map('intval', $_POST['checkbox']));
ТС, есть такая фраза, которая мне понравилась: «Прежде, чем что-либо писать — попробуйте сначала прочесть что-либо».
А Вам нужно читать, читать, и еще раз читать.
Пока что очень слабое понимание принципов эксплуатирования уязвимостей и очень низкое знание синтаксиса php. Получились антисоветы по большей части.
Отложите эту статью и вернитесь к ней через год-два. А за это время подтяните знания, и по истечению этого срока Вы сами будете смеяться над этим «произведением».
Дочитал до первого примера:

$number = intval($_GET['input_number']);
if ($number)
{
... выполняем SQL запрос ...
}


Понял, что раз автор не считает 0 годным числом, то дальше читать смысла нет. А также, тут не учтено, что GET-переменная input_number может быть не установлена.
Это лишь пример.
Во многих моих скриптах отсутствует 0, а точнее обозначает ошибку.
Only those users with full accounts are able to leave comments. Log in, please.