Нужно ли фильтровать данные file_exists(), file_get_contents()?

Question

[ex3xeng]

имеется такая схема:

$file = (isset($_GET['file']) && !empty($_GET['file']) ? $_GET['file'] : false);
if ($file) {
   if (file_exists($file)) $file = file_get_contents($file);
}

при этом параметр get отправляется клиентом, нужно ли как то фильтровать входящие данные в этом случае? возможны ли ошибки?

Answer 1

[Алексей Уколов]

Конечно, это классическая ошибка - вы фактически даете пользователю прочитать произвольный файл на вашем сервере. Это огномная дыра в безопасности.
На stepic.org есть курс по безопасности веб-проектов и в нем есть глава, посвященная именно этой проблеме.

Comments

[ex3xeng]

соответственно вопрос) как фильтровать то?
strip_tags? htmlspecialchars? какие могут быть ошибки?

проверял так, передавал в GET ' " / $ %^ & * ошибок не было) что я упустил?

[Алексей Уколов]

Лучше всего фильтровать по белому списку, проверять папку, запрещать '../' и еще много чего. Коротко говоря - не надо так делать вообще.

[ex3xeng]

Алексей Уколов: ой, немного не верно написал...

на самом деле я делаю так, if (file_exists("/files/price/".$file.".doc")) вот в такой форме можно?

[Алексей Уколов]

Всё-равно это опасно - то, что вы указываете в начале директории, легко обходится при помощи "../". Не уверен насчет расширения, но думаю, что и от него можно избавиться.

[ex3xeng]

Алексей Уколов: ну а если скажем проверять приходящую от пользователя строку preg_match на разрешенные символы, и разрешить только маленькие латинские буквы?

if (!только маленькие латинские буквы) 403

[Алексей Уколов]

Можно напридумывать кучу проверок, но нельзя поручиться, что их будет достаточно. А что мешает хранить пути к файлам в БД и отдавать их по ID?

[Adamos]

Необязательно в БД, но прослойка, нарушающая соответствие между запросом и именем файла, необходима. У меня по сайту шерстят китайские боты, которые КАЖДЫЙ параметр в обнаруженных ссылках с GET-полями проверяют на ../../../../.../etc/passwd, например.

[ex3xeng]

Алексей Уколов: Adamos: просто интересно, чем недостаточно preg_match тут ведь жесткая проверка по шаблону, если есть что то кроме маленьких латинских букв 403

Если не трудно ответьте на вышепоставленный вопрос, а на счет БД не хотелось бы дергать лишний раз БД

[Adamos]

ex3xeng: просто интересно, зачем солдат учат ходить в ногу, если в бою они так совершенно не ходят?
Отвечаем: смотрит программист на код, в котором пришедшие снаружи данные подставляются в путь к файлу - и либо у него сжимается сфинктер от ярости, либо он не программист.
Потому что каждый раз, возвращаясь через три года использования по-всякому к этому коду, нормальный программист будет крыть чем попало того, кто так написал, а не сделал, как принято у нормальных людей и не использовал потом этот же самый кусок кода каждый раз, когда перед ним снова встанет та же задача.

[ex3xeng]

Adamos: ну вот допустим такой кусок

file_exists("/files/omg/dir/".clear($_GET['file']).".php")

function clear($str) {
return preg_match($_GET['file']);
}

за что должны крыть то? есть некая функция которая используется везде.

Вопрос ведь стоит в том что выше челочек сказал что сколько бы проверок не было этого недостаточно, но чего в данной ситуации недостаточно не ясно

[Adamos]

ex3xeng: так это и есть очень серьезная ошибка начинающего программиста: решая частую проблему, приспосабливать решение к данной ситуации.
Есть выстраданный принцип безопасности: если GET-параметр никак не связан с именем ресурса на диске, он точно не создаст уязвимости. Ни в данной ситуации, ни в какой-либо иной. Если вы можете так сделать - будет хорошо. У вас есть достаточно весомые причины не делать хорошо, а ограничиться частным решением? Если да, то такое поведение простительно. Если нет - не идите на поводу у лени. Для будущего.

[ex3xeng]

Adamos: ну как бы и не хочется на поводу у лени идти)
понимаете проще сделать так:

mysli_real_escape_string(bla_bla_bla($_GET['file']));

SELECT FROM FILES WHERE и т.д но не хотелось бы дергать БД и ищу альтернативу, наиболее оптимизированный вариант что бы без нагрузок и соответственно безопасный.

вот поэтому и копаю... т.е в данной ситуации в принципе все работает? но я делаю не так как нужно? а как нужно?

[Adamos]

ex3xeng: давайте пофантазируем.
Итак, вы, получая запрос, обращаетесь к базе за именем файла, потом проверяете, есть ли этот файл, потом загружаете содержимое файла в память и отдаете пользователю. Вас в этом процессе почему-то пугает обращение к базе данных. Но если запрос простейший, то выполнен он будет чертовски быстро, а во второй раз - вообще почти моментально, потому что БД еще не успеет выкинуть эти данные из кэша. Проверка существования файла займет примерно то же время, но ее вы почему-то не боитесь. Основную массу времени, собственно, отнимут два следующих шага.
А "в данной ситуации в принципе все работает" - это первые слова в Святой Книге Говнокода.

Кстати, какого черта escape_string? Вы уже оторвались от имен файлов, почему не использовать числовые идентификаторы?

Ну, и смотрите: у вас теперь есть база данных, в которой хранятся те документы, которые смогут загружать пользователи. Можно связать с ней инструмент для загрузки таких файлов (безопасный, потому что файлы не будут доступны по имени!), инструмент вставки в материал на сайте ссылки на файл, оперирующий уже не его именем, а тем описанием, которое ему дали при загрузке... Начинаете чувствовать, что у вас энтерпрайз, а не наколенная поделка? Еще нет? Тогда продолжим!..

[Алексей Уколов]

Ну, я только подписаться под всем, что написал Adamos могу. :)

Answer 2

[Alexey Hog]

Некоторую информацию можно почерпнуть здесь