Jey Val Star

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

Recommended Posts

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

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

// определения языка сайта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)

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

Share this post


Link to post
Share on other sites

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

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

Share this post


Link to post
Share on other sites

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

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; //подключение языкового файла

Share this post


Link to post
Share on other sites

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

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

Share this post


Link to post
Share on other sites

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

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

Edited by Radiocity

Share this post


Link to post
Share on other sites

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

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

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

Share this post


Link to post
Share on other sites

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

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

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

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

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

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

Share this post


Link to post
Share on other sites

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

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

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

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

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

Share this post


Link to post
Share on other sites

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

// определения языка сайта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 by Jey Val Star

Share this post


Link to post
Share on other sites

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

Share this post


Link to post
Share on other sites

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

 

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

http://pastebin.com/yfkgLFzw

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

Share this post


Link to post
Share on other sites
Но все таки что это значит?

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

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

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

Edited by CoDy

Share this post


Link to post
Share on other sites

Вставлю и я своих 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>';    }}?>

Share this post


Link to post
Share on other sites
    $get_me_lang = array_search($lang, $lang_array);    if ($lang_array[$get_me_lang]) {
А где обработка случая, когда $get_me_lang === false?

Share this post


Link to post
Share on other sites

 

    $get_me_lang = array_search($lang, $lang_array);    if ($lang_array[$get_me_lang]) {
А где обработка случая, когда $get_me_lang === false?

 

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

Share this post


Link to post
Share on other sites

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

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

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

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

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

Share this post


Link to post
Share on other sites

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

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

Share this post


Link to post
Share on other sites
Как это спасёт от ситуации, когда искомый элемент идёт с нулевым индексом?

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

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

  • Member Statistics

    46,335
    Total Members
    3,128
    Most Online
    Витольд Магикан
    Newest Member
    Витольд Магикан
    Joined
  • Recently Browsing   0 members

    No registered users viewing this page.