Сообщений: 982
Тем: 73
Зарегистрирован: Jan 2009
Репутация:
2,862
MiR @ Aion Emu
Продам сборки v2.7 v3.9 v4.7 v4.8 v4.9 v5.1 , сборки мастер сервера v1.9 и v2.7 ,пишу скрипты и квесты на заказ , правки ядра , правки даты , писать в ПМ
Сообщений: 59
Тем: 4
Зарегистрирован: Jun 2012
Репутация:
187
Вообще не грамотно составленный скрипт, так как для всех чисел, особенно подставляемых в склейку MySQL запроса, нужно использовать как минимум intval(), а еще лучше пользоваться приведением типов и приводить к числу.
Теперь вкратце по самому скрипту.
PHP код: <?php
if ($type == 2)
{
$count = $top_bonus['count_sms'];
}
else
{
$count = $top_bonus['count_site'];
}
Не смог проследить переменную $top_bonus, она видимо находится в config/config.php, импортируемый почти в самом начале. Но в любом случае я бы сделал как минимум вот так:
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 код: <?php
$sql = mysql_query("SELECT * FROM `top_vote` WHERE `name` = '".$player."' AND `time` = '".$date."';");
Через переменную $date можно провести SQL инъекцию, если попадется недобросовестный топ серверов или просто будет подменена ссылка на список проголосовавших. Почему? Сейчас объясню. Разберемся с происхождением ссылки (выстрою ввиде кода с комментариями):
PHP код: <?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 окажутся ошибки со страницы.
Сообщений: 509
Тем: 7
Зарегистрирован: Apr 2008
Репутация:
1,660
Пользуйтесь prepared statements из mysqli, не собирайте запросы строками. Это позволит отсечь большинство дыр в скрипте.
Сообщений: 118
Тем: 4
Зарегистрирован: Jun 2011
Репутация:
469
В общем, если говорить о дырках, то она одна - это автор скрипта (не желаю обидеть, хочу просто указать на некомпетентность), говоря о Pocan - могу сказать тоже самое. Хотя, если говорить об инъекции, которая видна не вооруженным взглядом, то он прав, но не совсем: на сколько я знаю, данная бага есть в любой обвязке для lineage, как пример - stressweb. На сколько я знаю при голосовании можно указать лишь ник, остальное устанавливается автоматически скриптом сервера (в вашем случае остальное это date и type), а ник в свою очередь проходит фильтрацию на стороне топа, т.е. получается, что уязвимостью могут воспользоватся лишь владельцы топа либо же те, кто получил полный доступ к вашему хостингу (хотя тут проводить инъекцию уже нет смысла ).
Хотелось бы отметить крайнюю, ну просто крайнюю неграмотность писавшего код ещё раз, т.к. скрипт скорее всего работать будет, но говнокоднее говнокода я давно не видел. Напоминает мне:
PHP код: <?php
$query = "DROP TABLE tablica";
if (mysql_query($query))
echo 'Таблица существует';
Сообщений: 187
Тем: 6
Зарегистрирован: Jun 2012
Репутация:
108
x3k Написал:В общем, если говорить о дырках, то она одна - это автор скрипта (не желаю обидеть, хочу просто указать на некомпетентность), говоря о Pocan - могу сказать тоже самое. Хотя, если говорить об инъекции, которая видна не вооруженным взглядом, то он прав, но не совсем: на сколько я знаю, данная бага есть в любой обвязке для lineage, как пример - stressweb. На сколько я знаю при голосовании можно указать лишь ник, остальное устанавливается автоматически скриптом сервера (в вашем случае остальное это date и type), а ник в свою очередь проходит фильтрацию на стороне топа, т.е. получается, что уязвимостью могут воспользоватся лишь владельцы топа либо же те, кто получил полный доступ к вашему хостингу (хотя тут проводить инъекцию уже нет смысла ).
Хотелось бы отметить крайнюю, ну просто крайнюю неграмотность писавшего код ещё раз, т.к. скрипт скорее всего работать будет, но говнокоднее говнокода я давно не видел. Напоминает мне:
PHP код: <?php
$query = "DROP TABLE tablica";
if (mysql_query($query))
echo 'Таблица существует';
Ну так исправьте же недоразумение, о люсера кодерь
// GPRS удалил подпись пользователя
Сообщений: 59
Тем: 4
Зарегистрирован: Jun 2012
Репутация:
187
07-26-2012, 10:34 PM
(Сообщение последний раз редактировалось: 07-27-2012, 03:52 PM Pocan.)
Aquanox Написал:Пользуйтесь prepared statements из mysqli, не собирайте запросы строками. Это позволит отсечь большинство дыр в скрипте. Безусловно, использовать mysqli или pdo - это решение от SQL инъекций, но ведь начинающие php-программисты же должны знать, что происходит за плейсхолдерами.
Да и подлезть сейчас можно не только через SQL инъекцию, способов много. Один лишь переход на mysqli не даст сильного эффекта безопасности. Хотя касательно этого скрипта - действительно отсечется большинство дыр, но это скорее частный случай.
Напишу еще один момент - переход ( именно переход, а не написание с нуля), например, на mysqli - это далеко не простая процедура для тех, кто пишет подобные этому топики (так как нужно будет написать новую обертку и перелопатить половину обвязки для замены склеенных запросов на запросы с плейсхолдерами), поэтому почему бы им вначале не разобраться с php_mysql, а потом уже переходить на более усовершенствованные технологии. Хоть разработчики php и хотят прекратить поддержку php_mysql, я думаю это еще будет не скоро.
x3k Написал:В общем, если говорить о дырках, то она одна - это автор скрипта (не желаю обидеть, хочу просто указать на некомпетентность), говоря о Pocan - могу сказать тоже самое. Подождите-ка, уважаемый x3k.
В чем выражается моя некомпетентность? Давайте конкретнее или для вас привычно бросаться необоснованными обвинениями во всех подряд?
Давайте разберемся подробнее. Топик Стартер, а именно Romanz, четко написал:
Цитата:подскажите есть ли в данном скрипте дырка
Обратите внимание, что он не писал ничего, типа "перепишите мой скрипт с нуля с использованием, например, mysqli"(что я бы мог спокойно сделать, но не за просто так), а он написал именно то, что я выделил в цитату. Можете даже перечитать его сообщение, если не верите мне.
Я в самом начале своего поста написал:
Цитата:Вообще не грамотно составленный скрипт
И начал комментировать дырки конкретно в этом скрипте.
Что я сделал, по вашему мнению, не компетентно?
Добавлено через 10 часов 54 минуты
x3k Написал:Хотелось бы отметить крайнюю, ну просто крайнюю неграмотность писавшего код ещё раз, т.к. скрипт скорее всего работать будет, но говнокоднее говнокода я давно не видел. Не знаю как вам, а мне кажется, что настоящий фанат своего дела должен разбираться во всем, что связано с его специализацией (сейчас речь идет в частности про php) и не должен отсортировать, что является плохим кодом, а что хорошим. Скажу более того, плохой код можно быстро превратить в хороший.
x3k Написал:Хотя, если говорить об инъекции, которая видна не вооруженным взглядом, то он прав, но не совсем: на сколько я знаю, данная бага есть в любой обвязке для lineage, как пример - stressweb. На сколько я знаю при голосовании можно указать лишь ник, остальное устанавливается автоматически скриптом сервера (в вашем случае остальное это date и type), а ник в свою очередь проходит фильтрацию на стороне топа, т.е. получается, что уязвимостью могут воспользоватся лишь владельцы топа либо же те, кто получил полный доступ к вашему хостингу (хотя тут проводить инъекцию уже нет смысла ). Вы так же не учли момент, если у вас вдруг уведут или сбрутят пару логин/пароль от админ панели и просто подменят ссылку. Это в принципе может быть с большим шансом, чем получить инъекцию через известные топы. Но это скорее отступление от темы.
Хотелось бы еще раз уточнить, что я не писал свой код, я лишь "подсказал дырки, которые есть в данном скрипте" и навел на мысль, как их исправить.
А вы, уважаемый x3k, оскорбили меня, обвинив в некомпетентности еще и без каких-либо пояснений.
Я прислушиваюсь к правильной критике, так как мы все люди, все ошибаемся и кто-то знает больше в одном направлении, кто-то в другом, но сейчас, я считаю, совсем не тот случай.
Я жду пояснений и аргументов от вас, x3k.
Сообщений: 118
Тем: 4
Зарегистрирован: Jun 2011
Репутация:
469
Ух ты, слово некомпетентен уже считается оскорблением? Я имел в виду, что раз вы указали это как уязвимость, то скорее всего вам не приходилось работать в данной сфере и с топами, т.к. фильтровать данные значения практически тоже самое, что и $_SESSION. Ну, в общем, что-то вроде того
Сообщений: 59
Тем: 4
Зарегистрирован: Jun 2012
Репутация:
187
x3k Написал:Ух ты, слово некомпетентен уже считается оскорблением? Я фанат своего дела и считаю обвинение в некомпетентности - оскорблением (особенно когда это не является правдой).
x3k Написал:Я имел в виду, что раз вы указали это как уязвимость, то скорее всего вам не приходилось работать в данной сфере и с топами Вы ошибаетесь, я работал и с топами, и в этой сфере около года, работал с разными сборками, причем приходилось использовать не только php (с mysql, jquery), приходилось писать свои ивенты на java, переделывать квесты на python.
Да и вообще, откуда бы я знал MMOTOP и L2Top, если бы не знал про топы и не работал с ними и для чего бы я написал вот это:
Цитата:Ну, хотя, если данный скрипт предназначен для топов типа L2Top или MMOTop, то SQL инъекцию врятли кто-то будет внедрять через этот файл
А насчет
x3k Написал:фильтровать данные значения практически тоже самое, что и $_SESSION Если честно, я уже начал сомневаться в вашей компетентности, так как сессионная переменная уж точно не является безопасной на 100% если вы не можете проследить все её использование по скрипту или не уверены в помещаемых в нее данных. Приведу пример. На одной странице сессионной переменной присваивается значение из БД (например поле varchar, text or etc., которое заполняется из вне), на другой странице делается вывод этой переменной, как вы написали, без фильтра. Последствия этого "правильного скрипта" озвучить или сами догадаетесь?
Я уже молчу про register_globals on.
Походу вы маловато программировали на php, если так безрассудно рассуждаете про сессии.
Сообщений: 118
Тем: 4
Зарегистрирован: Jun 2011
Репутация:
469
Pocan Написал:Я фанат своего дела и считаю обвинение в некомпетентности - оскорблением (особенно когда это не является правдой).
Вы ошибаетесь, я работал и с топами, и в этой сфере около года, работал с разными сборками, причем приходилось использовать не только php (с mysql, jquery), приходилось писать свои ивенты на java, переделывать квесты на python.
Да и вообще, откуда бы я знал MMOTOP и L2Top, если бы не знал про топы и не работал с ними и для чего бы я написал вот это:
А насчет
Если честно, я уже начал сомневаться в вашей компетентности, так как сессионная переменная уж точно не является безопасной на 100% если вы не можете проследить все её использование по скрипту или не уверены в помещаемых в нее данных. Приведу пример. На одной странице сессионной переменной присваивается значение из БД (например поле varchar, text or etc., которое заполняется из вне), на другой странице делается вывод этой переменной, как вы написали, без фильтра. Последствия этого "правильного скрипта" озвучить или сами догадаетесь?
Я уже молчу про register_globals on.
Походу вы маловато программировали на php, если так безрассудно рассуждаете про сессии.
окей, все понятно. Уволены. При чем тут фильтрование сессии, если фильтровать нужно там, где "вводится из вне". Сессии хранятся на стороне сервера и никто их редактировать кроме вас не может. Так нафига тогда их фильтровать, если я сам присваиваю им значения? Давайте тогда фильтровать любую присваиваемую переменную.
Далее по тексту, если вы работали с топами, то какого спрашивается вы пишете человеку об уязвимостях, которых быть не может.
Сообщений: 1,832
Тем: 50
Зарегистрирован: Oct 2009
Репутация:
62,283
Зачем паролю mysql_escape_string если он в base64 хранится? почему я не могу использовать кавычки в своих паролях? зачем так делать то
|