Что не так в этом коде PHP

[Jey Val Star]

Здравствуйте.

Есть участок кода определяющий какой язык выбрал пользователь

// определения языка сайтаif($_GET['lang']){    if(file_exists('languages/' . $_GET['lang'] . '.php'))    {        $lang_file = $_GET['lang'] . '.php';        $lang_link = '?lang=' . $_GET['lang'];    }}include 'languages/' . $lang_file; //подключение языкового файла

Такой ответ от программиста

 

 

Даете почти прямой доступ к файловой системе сервера (engine/engine.php:13 и :19)

Вопрос - как правильно выполнить этот код?

[rus]

http://php.net/manual/ru/security.filesystem.php

[Jey Val Star]

Все равно, мало чего понял...

Я не удаляю файлы, а подключаю притом с проверкой.

[rus]

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

по большому счету, настройки прав на сервере вполне хватит для предотвращения прямого доступа к файловой системе.

[Jey Val Star]

Может тут имелось в виду указание пути через переменную (константу)?

define('LANG','languages');// определения языка сайтаif($_GET['lang']){    if(file_exists(LANG . $_GET['lang'] . '.php'))    {        $lang_file = $_GET['lang'] . '.php';        $lang_link = '?lang=' . $_GET['lang'];    }}include LANG . $lang_file; //подключение языкового файла

[Galaxy]

Ну тут наверное имелось в виду что в $_GET['lang'] может всякое приходить. Как вариант можно получить массив файлов директории, и проверять на наличие $_GET['lang'] в этом массиве и если такое существует то инклудить файл.

Но как сказал rus опасности в этом коде нет, и намного правильнее настраивать права на сервере.

[Jey Val Star]

Но все таки что это значит?

 

 

Даете почти прямой доступ к файловой системе сервера

[Radiocity]

Потому что в $_GET['lang'] можно послать любое имя файла, и при определенном навыке получить полный контроль над сайтом и базой данных.

Первое, на что проверяют сайт при взломе - это защищенность $_GET параметров, а также возможность провести SQL инъекции через них.

Edited January 10, 2015 by Radiocity

[bryknyk]

Вспоминая первые шаги - любому новичку должна попадаться статья на офф сайте пыхи, на форуме пхпклаб и т.п. Это просто классический пример инклюдинга - его суют везде куда не лень. Вопрос, что вы не можете знать на каком сервере будет выполняться код и программист прав. Кроме того, этот код неправилен от и до. Сборка пути разбросана по коду, если нужно будет изменить путь - придеться лезть в логику. есть дубирование кода - $_GET['lang'] . '.php' выполняется два раза! и вообще - 4 (!) раза обращение к $_GET['lang'] - кроме дубрилования, т.к. 'lang' это строка не исключено, что при изменении придется искать и заменять и если в коде будет переменная $lang, то без регекспов появится сюрприз.

Кроме того, что будет с переменной $lang_file если if не сработает. Или это подразумевался сокращенный вариант if и переменная уже инициализирована где-то в коде по дефолту, или это просто недочет и там ничего не будет, но будет ошибка.

rus вам скинул ссылку- там ключевой момент - изменение пользователем содержимого массивов пост или гет для изменения пути подключаемого файла!

[Jey Val Star]

Вспоминая первые шаги - любому новичку должна попадаться статья на офф сайте пыхи, на форуме пхпклаб и т.п. Это просто классический пример инклюдинга - его суют везде куда не лень. Вопрос, что вы не можете знать на каком сервере будет выполняться код и программист прав. Кроме того, этот код неправилен от и до. Сборка пути разбросана по коду, если нужно будет изменить путь - придеться лезть в логику. есть дубирование кода - $_GET['lang'] . '.php' выполняется два раза! и вообще - 4 (!) раза обращение к $_GET['lang'] - кроме дубрилования, т.к. 'lang' это строка не исключено, что при изменении придется искать и заменять и если в коде будет переменная $lang, то без регекспов появится сюрприз.

Кроме того, что будет с переменной $lang_file если if не сработает. Или это подразумевался сокращенный вариант if и переменная уже инициализирована где-то в коде по дефолту, или это просто недочет и там ничего не будет, но будет ошибка.

rus вам скинул ссылку- там ключевой момент - изменение пользователем содержимого массивов пост или гет для изменения пути подключаемого файла!

Если я правильно вас понял то $_GET['lang'] (если существует) нужно проверить на допустимость при этом перевести в переменную. А потом использовать эту переменную для includ. И следующий момент - все пути прописать в конфиге (к примеру в виде констант) и для подключения файлов использовать эти константы?

что будет с переменной $lang_file если if не сработает

Эта переменная определена выше. В этом участке просто переопределяется значение (если есть).

[Switch74]

Я соглашусь с программистом.
Предположим у вас в корне есть файл password.php (этот файл не обязательно может хранить пароли и называться так), но

я передаю $_GET['lang'] = "/../password" и вуаля, мы видим то, чего не должны были, ну или хотя бы можем определить наличие интересующего нас файла.

а если я сделаю такое $_GET['lang'] = "/../password.ini?" тут я вообще любой файлик могу получить

1. совет, который обычно всегда дают - это проверять все входящие данные и даже куки

2. я бы посоветовал вам использовать свою функцию include в которой бы проводилась проверка прав доступа к файлу (если у вас существует система прав доступа к файлам)

[Jey Val Star]

Переделал код - так лучше?

// определения языка сайтаif(isset($_GET['lang'])){    $key = array_search($_GET['lang'], $lang_file_arr); // проверка допустимых значений в GET    $lang_file = $lang_file_arr[$key]. '.php'; // для подключеия файла    $lang_link = '?lang=' . $lang_file_arr[$key]; // установка языка для ссылок}include PATH_LANG . $lang_file; //подключение языкового файла

PS опробовал передать в GET вызов другого файла (../index.php) вышло сообщение о ошибке "Прекращена работа программы Apache ...". Теперь не выводит)))

Edited January 10, 2015 by Jey Val Star

[Switch74]

мне кажется у вас должен быть $key  по умолчанию какой-то, который будет грузиться при отсутствии или не правильном $_GET['lang']

[Быколай]

ну, ронять своим кодом apache это не есть гут =)

 

мой многословный вариант реализации:

http://pastebin.com/yfkgLFzw

не запускал, так что это не рабочий пример, скорее в качестве демонстрации ООП подхода. Можно выделить интерфейс ( ::inludeLanguage($code) ) и кроме файловой реализации запилить любую другую совместимую.

[CoDy]

Но все таки что это значит?

Вот Вам пример урла, который может быть передан от пользователя:

http://host.com/request?lang=../../../script/scripthttp://host.com/request?lang=../../../lib/scripthttp://host.com/request?lang=../../site/download/my_file

Таким образом Вашь скрипт позволит "лазить" по всем доступным закаулкам ФС, которые доступны пользователю от имени которого исполняется скрипт. От Вас хотят исключить возможность вставки спец символов(".","..","/"), которые позволят выйти за пределы папки где лежат языковые скрипты.

Edited January 12, 2015 by CoDy

[Int]

http://php.net/basename, не? Ну или через pathinfo()

[pashkje]

Вставлю и я своих 5 копеек.

Я бы реализовал примерно так:

<?if ($_GET["LANG"]) {    $lang = htmlspecialchars($_GET["LANG"]);    $lang_array = array(); //массив со всеми возможными языками    $get_me_lang = array_search($lang, $lang_array);    if ($lang_array[$get_me_lang]) {        include 'languages/' . $lang_array[$get_me_lang];    }    else {        echo '<pre>'; print_r("ХАКИРШТОЛЕ?"); echo '</pre>';    }}?>

[Int]

    $get_me_lang = array_search($lang, $lang_array);    if ($lang_array[$get_me_lang]) {

А где обработка случая, когда $get_me_lang === false?

[pashkje]

 

    $get_me_lang = array_search($lang, $lang_array);    if ($lang_array[$get_me_lang]) {

А где обработка случая, когда $get_me_lang === false?

 

может чего-то я не понимаю, но я же написал else. Не могли бы объяснить. Спасибо

[Int]

может чего-то я не понимаю, но я же написал else. Не могли бы объяснить. Спасибо

1) В php 0 == '' == false == '0' == null.

2) Функция array_search ищет вхождение элемента в массив и возвращает его индекс.

3) Индекс может оказаться нулевым, это допустимое значение, насколько я понимаю

4) В случае, если элемент вообще не найден, то возвращается не ноль, а false, поэтому для if нужно строгое сравнение if ($lang_array[$get_me_lang] !== false) {} else {}, иначе получается, что в else уходит даже допустимое значение 0.

[rus]

Int, а не проще проверять на сам массив: if(is_array()) {} ?

[Int]

Int, а не проще проверять на сам массив: if(is_array()) {} ?

Как это спасёт от ситуации, когда искомый элемент идёт с нулевым индексом?

[rus]

Как это спасёт от ситуации, когда искомый элемент идёт с нулевым индексом?

мда, и в правду - никак.