Форум администраторов игровых серверов

- PHP (https://forum.zone-game.info/forumdisplay.php?f=125)

- - Подскажите по скрипту (https://forum.zone-game.info/showthread.php?t=21265)

Подскажите по скрипту

Доброго времени суток , ребят подскажите есть ли в данном скрипте дырка , а то что то подозрения есть , спасибо

PHP код:


		
			
<?php

if ($_POST && isset($_POST['action']) && $_POST['action'] === "give")

{

include ("config/config.php");

    mysql_query("SET NAMES cp1251");

    mysql_select_db($mysql_connect['dbl'], $connect);

    $acc = mysql_real_escape_string($_POST['account']);

    $pass = mysql_real_escape_string($_POST['password']);

    $r = mysql_query("SELECT password FROM `account_data` WHERE name='$acc';");

    if (mysql_num_rows($r) > 0){

        $row = mysql_fetch_array($r);

        if ((base64_encode(pack("H*", sha1(utf8_encode($pass))))) == $row['password']){

            mysql_select_db($mysql_connect['dbg']);

            $player = mysql_real_escape_string($_POST['player']);

            $r = mysql_query("SELECT `id` FROM `players` WHERE `name`='".$player."' AND `account_name`='".$acc."';");

            if (mysql_num_rows($r) > 0)

            {

                $id = mysql_fetch_assoc($r);

                $id = $id['id'];

                $ip = explode('.', $_SERVER['REMOTE_ADDR']);

                $ip = $ip[0] . '.' . $ip[1] . '.' . $ip[2] . '.' . $ip[3];

                $file = file($topfile);

                $i = 0;

                foreach ($file as $line)

                {

                    $parts = explode("\t", $line);

                    $date = $parts[1];

                    $name = $parts[3];

                    $type = $parts[4];

                    $search_name = (eregi($player, $name));

                    if(!empty($search_name))

                    {

                        $sql = mysql_query("SELECT * FROM `top_vote` WHERE `name` = '".$player."' AND `time` = '".$date."';");

                        $num_rows = mysql_num_rows($sql);

                        $check = false;

                        if ($num_rows == 0)    $check = true;            

                        if ($check)

                        {

                            $added = false;

                            if ($type == 2)

                            {

                                $count = $top_bonus['count_sms'];

                            }

                            else

                            {

                                $count = $top_bonus['count_site'];

                            }

                            $sql = mysql_query("SELECT `money` FROM `account_data` WHERE `name` = '".$acc."';");

                            if (mysql_num_rows($sql) > 0)

                            {

                                $sql = mysql_query("UPDATE `account_data` SET money=`money`+".$count." WHERE name ='".$acc."';");

                                $added = mysql_affected_rows();

                            }

                            else

                            {

                                $ins_id = $char['id'].rand(0,999);

                                $chk = mysql_fetch_row(mysql_query("SELECT `money` FROM `account_data` WHERE name = '".$acc."';"));

                                if($chk['0'] != '0') {

                                    $ins_id = $char['id'].rand(0,999);

                                }

                                $sql = mysql_query("INSERT INTO `account_data` SET money='".$count."', name='".$acc."';");

                                $added = mysql_affected_rows();

                            }

                            if($added)

                            {

                                mysql_query("INSERT INTO `top_vote` VALUES ('".$player."', '".$ip."', '".$date."', '".$type."');");

                                $i = $i+$count;

                            }

                        }

                    }

                    $text = "<span style='color:green'>Получено ".$i." поинтов</span><br>";

                }

            }

            else

            {

                $text = "<span style='color:red'>Персонаж не существует.</span><br>";

            }

        }

        else

        {

            $text = "<span style='color:red'>Ошибка при авторизации: Вы указали неверный пароль.</span><br>";

        }

    }

    else

    {

        $text = "<span style='color:red'>Ошибка при авторизации: Такой аккаунт не существует.</span><br>";

    }

    mysql_close($connect);

}

?>

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"

        "http://www.w3.org/TR/html4/loose.dtd">

<html>

<head>

    <title>Хочу награду за голосование</title>

    <link href="css/style.css" type="text/css" rel=stylesheet>

    <script src="js/jquery.js" type="text/javascript"></script>

    <script src="js/jquery.jNice.js" type="text/javascript"></script>

</head>

<body>



<script type="text/javascript">//<![CDATA[

function isAlphaNumeric(value) {

    return value.match(/^[a-zA-Z0-9]+$/_);

}

function checkform(f) {

    if (f.account.value == "") {

        alert("Не введен аккаунт");

        return false;

    }

    if (!isAlphaNumeric(f.account.value)) {

        alert("Некорректный логин! Используйте только символы и цифры!");

        return false;

    }

    if (f.password.value == "") {

        alert("Не введен пароль");

        return false;

    }

    if (!isAlphaNumeric(f.password.value)) {

        alert("Некорректный пароль! Используйте только символы и цифры!");

        return false;

    }

    if (f.player.value == "") {

        alert("Не введено имя игрока");

        return false;

    }

    return true;

}//]]>

</script>



<div id="wrapper">

    <div class="tooltip"></div>

    <form class="jNice" action="" id="form" method="post" onsubmit="return checkform(this)">

        <table cellSpacing="0" cellPadding="3" align="center" width="400px"  height="80" border="0">

            <tbody>

            <tr>

                <td align="right" width="100"><strong>Логин</strong>:</td>

                <td><input id="account" size=30 name="account" title="Ваш аккаунт"/></td>

            </tr>

            <tr>

                <td align="right"><strong>Пароль</strong>:</td>

                <td><input id="password" type="password" size=30 name="password" title="Ваш пароль"/></td>

            </tr>

            <tr>

                <td align="right"><strong>Имя игрока</strong>:</td>

                <td><input id="player" size=30 name="player" title="Имя игрока за которого хотите получить бонус"/></td>

            </tr>

            <tr>

                <td><input type="hidden" name="action" value="give"></td>

                <td><input id="submit" type="submit" value="Получить" title="Получить ваш бонус за голосование""/>

                    <input type="reset" value="Сброс" title="Сброс полей"/>

                </td>

            </tr>

            <tr>

                <td></td>

            </tr>

            <tr>

                <td colspan="2"><center><?php echo $text ?></center></td>

            </tr>

            </tbody>

        </table>

    </form>

    <script type="text/javascript">

        $("#form :input").tooltip({

            position: "center right",

            offset: [-2, 10],

            effect: "fade",

            opacity: 0.7,

            tip: '.tooltip'

        });

    </script>



</div>

</body>

</html>

Re: Подскажите по скрипту

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

Теперь вкратце по самому скрипту.

PHP код:


		
			
if ($type == 2) 

{ 

     $count = $top_bonus['count_sms']; 

} 

else 

{ 

     $count = $top_bonus['count_site']; 

}

Не смог проследить переменную $top_bonus, она видимо находится в config/config.php, импортируемый почти в самом начале. Но в любом случае я бы сделал как минимум вот так:

PHP код:


		
			
if ($type == 2) 

{ 

     $count = (int)$top_bonus['count_sms']; 

} 

else 

{ 

     $count = (int)$top_bonus['count_site']; 

}

Но если есть 100% уверенность, что $top_bonus['count_sms'] и $top_bonus['count_site'] - числа, то в принципе можно оставить все как есть в этом куске кода.

Но есть еще более опасный момент вот с этим запросом:

PHP код:


		
			
$sql = mysql_query("SELECT * FROM `top_vote` WHERE `name` = '".$player."' AND `time` = '".$date."';");

Через переменную $date можно провести SQL инъекцию, если попадется недобросовестный топ серверов или просто будет подменена ссылка на список проголосовавших. Почему? Сейчас объясню. Разберемся с происхождением ссылки (выстрою ввиде кода с комментариями):

PHP код:


		
			
//читаем файл $topfile и записываем каждую его строчку в массив

$file = file($topfile); 

//каждую строку рассматриваем отдельно

foreach ($file as $line)

{

   //режем строку по табу

   $parts = explode("\t", $line); 

   //расфасовываем куски строки по переменным

   $date = $parts[1];  //но ведь не факт, что тут будет число, а вдруг недоброжелатель подставит тут кусок SQL запроса?

   $name = $parts[3];  //тут тоже не известно, что будет, поэтому лучше фильтровать получаемые данные или проверять (и то, и то можно сделать, например, при помощи регулярок)

   $type = $parts[4];  //аналогично

}

Ну, хотя, если данный скрипт предназначен для топов типа L2Top или MMOTop, то SQL инъекцию врятли кто-то будет внедрять через этот файл, но иногда сайт топа может не загрузиться этим скриптом и тогда в переменной $file окажутся ошибки со страницы.

Re: Подскажите по скрипту

Пользуйтесь prepared statements из mysqli, не собирайте запросы строками. Это позволит отсечь большинство дыр в скрипте.

Re: Подскажите по скрипту

В общем, если говорить о дырках, то она одна - это автор скрипта (не желаю обидеть, хочу просто указать на некомпетентность), говоря о Pocan - могу сказать тоже самое. Хотя, если говорить об инъекции, которая видна не вооруженным взглядом, то он прав, но не совсем: на сколько я знаю, данная бага есть в любой обвязке для lineage, как пример - stressweb. На сколько я знаю при голосовании можно указать лишь ник, остальное устанавливается автоматически скриптом сервера (в вашем случае остальное это date и type), а ник в свою очередь проходит фильтрацию на стороне топа, т.е. получается, что уязвимостью могут воспользоватся лишь владельцы топа либо же те, кто получил полный доступ к вашему хостингу (хотя тут проводить инъекцию уже нет смысла :)).
Хотелось бы отметить крайнюю, ну просто крайнюю неграмотность писавшего код ещё раз, т.к. скрипт скорее всего работать будет, но говнокоднее говнокода я давно не видел. Напоминает мне:

PHP код:


		
			
$query = "DROP TABLE tablica";
if (mysql_query($query))
echo 'Таблица существует';

Re: Подскажите по скрипту

Цитата:

Сообщение от x3k (Сообщение 208464)

PHP код:


		
			
$query = "DROP TABLE tablica";

if (mysql_query($query))

echo 'Таблица существует';

Ну так исправьте же недоразумение, о люсера кодерь

Re: Подскажите по скрипту

Цитата:

Сообщение от Aquanox (Сообщение 208441)

Пользуйтесь prepared statements из mysqli, не собирайте запросы строками. Это позволит отсечь большинство дыр в скрипте.

Безусловно, использовать mysqli или pdo - это решение от SQL инъекций, но ведь начинающие php-программисты же должны знать, что происходит за плейсхолдерами.
Да и подлезть сейчас можно не только через SQL инъекцию, способов много. Один лишь переход на mysqli не даст сильного эффекта безопасности. Хотя касательно этого скрипта - действительно отсечется большинство дыр, но это скорее частный случай.
Напишу еще один момент - переход (именно переход, а не написание с нуля), например, на mysqli - это далеко не простая процедура для тех, кто пишет подобные этому топики (так как нужно будет написать новую обертку и перелопатить половину обвязки для замены склеенных запросов на запросы с плейсхолдерами), поэтому почему бы им вначале не разобраться с php_mysql, а потом уже переходить на более усовершенствованные технологии. Хоть разработчики php и хотят прекратить поддержку php_mysql, я думаю это еще будет не скоро.

Цитата:

Сообщение от x3k (Сообщение 208464)

В общем, если говорить о дырках, то она одна - это автор скрипта (не желаю обидеть, хочу просто указать на некомпетентность), говоря о Pocan - могу сказать тоже самое.

Подождите-ка, уважаемый x3k.
В чем выражается моя некомпетентность? Давайте конкретнее или для вас привычно бросаться необоснованными обвинениями во всех подряд?
Давайте разберемся подробнее. Топик Стартер, а именно Romanz, четко написал:

Цитата:

подскажите есть ли в данном скрипте дырка

Обратите внимание, что он не писал ничего, типа "перепишите мой скрипт с нуля с использованием, например, mysqli"(что я бы мог спокойно сделать, но не за просто так), а он написал именно то, что я выделил в цитату. Можете даже перечитать его сообщение, если не верите мне.
Я в самом начале своего поста написал:

Цитата:

Вообще не грамотно составленный скрипт

И начал комментировать дырки конкретно в этом скрипте.
Что я сделал, по вашему мнению, не компетентно?

Добавлено через 10 часов 54 минуты

Цитата:

Сообщение от x3k (Сообщение 208464)

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

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

Цитата:

Сообщение от x3k (Сообщение 208464)

Хотя, если говорить об инъекции, которая видна не вооруженным взглядом, то он прав, но не совсем: на сколько я знаю, данная бага есть в любой обвязке для lineage, как пример - stressweb. На сколько я знаю при голосовании можно указать лишь ник, остальное устанавливается автоматически скриптом сервера (в вашем случае остальное это date и type), а ник в свою очередь проходит фильтрацию на стороне топа, т.е. получается, что уязвимостью могут воспользоватся лишь владельцы топа либо же те, кто получил полный доступ к вашему хостингу (хотя тут проводить инъекцию уже нет смысла ).

Вы так же не учли момент, если у вас вдруг уведут или сбрутят пару логин/пароль от админ панели и просто подменят ссылку. Это в принципе может быть с большим шансом, чем получить инъекцию через известные топы. Но это скорее отступление от темы.

Хотелось бы еще раз уточнить, что я не писал свой код, я лишь "подсказал дырки, которые есть в данном скрипте" и навел на мысль, как их исправить.
А вы, уважаемый x3k, оскорбили меня, обвинив в некомпетентности еще и без каких-либо пояснений.
Я прислушиваюсь к правильной критике, так как мы все люди, все ошибаемся и кто-то знает больше в одном направлении, кто-то в другом, но сейчас, я считаю, совсем не тот случай.
Я жду пояснений и аргументов от вас, x3k.

Re: Подскажите по скрипту

Ух ты, слово некомпетентен уже считается оскорблением? Я имел в виду, что раз вы указали это как уязвимость, то скорее всего вам не приходилось работать в данной сфере и с топами, т.к. фильтровать данные значения практически тоже самое, что и $_SESSION. Ну, в общем, что-то вроде того :)

Re: Подскажите по скрипту

Цитата:

Сообщение от x3k (Сообщение 209064)

Ух ты, слово некомпетентен уже считается оскорблением?

Я фанат своего дела и считаю обвинение в некомпетентности - оскорблением (особенно когда это не является правдой).

Цитата:

Сообщение от x3k (Сообщение 209064)

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

Вы ошибаетесь, я работал и с топами, и в этой сфере около года, работал с разными сборками, причем приходилось использовать не только php (с mysql, jquery), приходилось писать свои ивенты на java, переделывать квесты на python.
Да и вообще, откуда бы я знал MMOTOP и L2Top, если бы не знал про топы и не работал с ними и для чего бы я написал вот это:

Цитата:

Ну, хотя, если данный скрипт предназначен для топов типа L2Top или MMOTop, то SQL инъекцию врятли кто-то будет внедрять через этот файл

А насчет

Цитата:

Сообщение от x3k (Сообщение 209064)

фильтровать данные значения практически тоже самое, что и $_SESSION

Если честно, я уже начал сомневаться в вашей компетентности, так как сессионная переменная уж точно не является безопасной на 100% если вы не можете проследить все её использование по скрипту или не уверены в помещаемых в нее данных. Приведу пример. На одной странице сессионной переменной присваивается значение из БД (например поле varchar, text or etc., которое заполняется из вне), на другой странице делается вывод этой переменной, как вы написали, без фильтра. Последствия этого "правильного скрипта" озвучить или сами догадаетесь?
Я уже молчу про register_globals on.
Походу вы маловато программировали на php, если так безрассудно рассуждаете про сессии.

Re: Подскажите по скрипту

Цитата:

Сообщение от Pocan (Сообщение 209179)

Я фанат своего дела и считаю обвинение в некомпетентности - оскорблением (особенно когда это не является правдой).

Вы ошибаетесь, я работал и с топами, и в этой сфере около года, работал с разными сборками, причем приходилось использовать не только php (с mysql, jquery), приходилось писать свои ивенты на java, переделывать квесты на python.
Да и вообще, откуда бы я знал MMOTOP и L2Top, если бы не знал про топы и не работал с ними и для чего бы я написал вот это:

А насчет

Если честно, я уже начал сомневаться в вашей компетентности, так как сессионная переменная уж точно не является безопасной на 100% если вы не можете проследить все её использование по скрипту или не уверены в помещаемых в нее данных. Приведу пример. На одной странице сессионной переменной присваивается значение из БД (например поле varchar, text or etc., которое заполняется из вне), на другой странице делается вывод этой переменной, как вы написали, без фильтра. Последствия этого "правильного скрипта" озвучить или сами догадаетесь?
Я уже молчу про register_globals on.
Походу вы маловато программировали на php, если так безрассудно рассуждаете про сессии.

:D окей, все понятно. Уволены. При чем тут фильтрование сессии, если фильтровать нужно там, где "вводится из вне". Сессии хранятся на стороне сервера и никто их редактировать кроме вас не может. Так нафига тогда их фильтровать, если я сам присваиваю им значения? Давайте тогда фильтровать любую присваиваемую переменную.

Далее по тексту, если вы работали с топами, то какого спрашивается вы пишете человеку об уязвимостях, которых быть не может.

Re: Подскажите по скрипту

Зачем паролю mysql_escape_string если он в base64 хранится? почему я не могу использовать кавычки в своих паролях? зачем так делать то:(

Re: Подскажите по скрипту

беру все свои слова назад. :)

Re: Подскажите по скрипту

Цитата:

Сообщение от ReaM (Сообщение 209421)

Это не уязвимость (или дыра, как назвал Топик Стартер), а скорее это один из недочетов данного скрипта. Но, в принципе, ваша правда. Но тут можно придраться не только к этому, тут много что некорректно составлено.

Цитата:

Сообщение от x3k

Сессии хранятся на стороне сервера и никто их редактировать кроме вас не может.

Совет на будущее - никогда не полагайтесь только на это, когда-нибудь это вас подведет.

Цитата:

Сообщение от x3k

Далее по тексту, если вы работали с топами, то какого спрашивается вы пишете человеку об уязвимостях, которых быть не может.

Точнее сказать "уязвимость, через которую маловернотно получить взлом". Но ведь эта уязвимость есть, поэтому я и написал про нее.

Цитата:

Сообщение от x3k

беру все свои слова назад. :)

Тогда считаю, что конфликт исчерпан. :)