Laravel по-русски

Русское сообщество разработки на PHP-фреймворке Laravel.

Ты не вошёл. Вход тут.

#1 30.03.2018 02:56:29

Безопасность кода

Здравствуйте. Я дописал один из проектов на гитхабе и может быть увидите, где есть уязвимость для любых атак? Заранее спасибо.
Часть из routes:

Route::post( 'shop/purchase/{shop}', 'Front\ShopController@postPurchase' );

Представление:

@foreach( $items as $item )
<form id="item_buy{{$item->id}}" action="{{ url( 'shop/purchase/' . $item->id ) }}" method="post" >
	{!! csrf_field() !!}
	<div class='news'>
		<div class='shop_img'>
			<a href="#"> <img src="{{ asset( File::exists( base_path( 'public/img/icons/' . $item->item_id . '.gif' ) ) ? 'img/icons/' . $item->item_id . '.gif' : 'img/icons/0.gif' ) }}" alt="{{ $item->name }}"></a>
			<br> <br>
			<b>Quantity, pcs.</b><br>
			<input class="count" name="count" type="number" value="1" min="1" max="{{ $item->max_count }}">
			<div></div>
		</div>
		<div class='news_content'>
			<div class='news_title'>
				<a href="#">{{ $item->name }}</a>
			</div>
			<div class='news_text'>
				{!! $item->description !!}
			</div>
			<div class='news_info'>
				Price: <b>{{ $item->price}}</b> {{ settings( 'currency_name' ) }}
			</div>
			<div id="news_button">                
				<a href="#" onclick="if(confirm('Are you sure you want to buy {{$item->name}}?\nYour current character is {{ Auth::user()->characterId() ? Auth::user()->characterName() : 'not selected.' }}.')){document.getElementById('item_buy{{$item->id}}').submit();}">{{ trans( 'shop.buy' ) }}</a>
			</div>
			<noscript>
				<input type="submit" value="{{ trans( 'shop.buy' ) }}" />
			</noscript>
			</span>
		</div>
	</div>
</form>

Метод в контроллере:

public function postPurchase( Request $request, ShopItem $item )
    {
        $validator = Validator::make( $request->all(), [
            'count' => 'required|numeric|min:1|max:9999'
        ]);

        if ( $validator->fails() )
        {
            return redirect( 'shop' )
                ->withErrors( $validator )
                ->withInput();
        }

        $count = $request->input('count');
        $user = Auth::user();
        $item_price = ( $item->discount > 0 ) ? $item->price - ( $item->price / 100 * $item->discount ) : $item->price;
        $item_price = $item_price * $count;
        
        if ( $user->money >= $item_price  )
        {
            if ($item->max_count >= $count)
            {
                $api = new API();
                $mail = array(
                    'title' => trans( 'shop.mail_item.title', ['name' => settings( 'server_name' )] ),
                    'message' => trans( 'shop.mail_item.message', ['name' => $item->name, 'count' => $item->count, 'staff' => settings( 'server_name' )] ),
                    'money' => 0,
                    'item' => array(
                        'id' => $item->item_id,
                        'pos' => 0,
                        'count' => $count,
                        'max_count' => $item->max_count,
                        'data' => $item->octet,
                        'proctype' => $item->protection_type,
                        'expire_date' => $item->expire_date,
                        'guid1' => 0,
                        'guid2' => 0,
                        'mask' => $item->mask,
                    ),
                );
                $api->sendMail( Auth::user()->characterId(), $mail['title'], $mail['message'], $mail['item'], $mail['money'] );
                $user->money = $user->money - $item_price;
                $user->save();
                flash()->success( trans( 'shop.purchase_complete', ['name' => $item->name] ) );
            }
            else
            {
                flash()->error( 'The amount is too large.' );
            }
        }
       
        else
        {
            flash()->error( trans( 'main.not_enough', ['currency' => strtolower( settings( 'currency_name' ) )] ) );
        }
        return redirect()->back();
    }

Основная суть: Пользователь может покупать предметы, которые будут отсылаться в игру. Можно указать количество и, если денег достаточно, они отправляются.

Не в сети

#2 30.03.2018 12:08:58

Re: Безопасность кода

if ( $user->money >= $item_price  )

Вроде раньше в играх такой баг назывался "дюп".
Небезопасная для потоков проверка (Non Thread Safe).
Если у меня money = 100, а $item_price = 100. Я с легкостью могу купить хоть 10 таких итемов. Запустив кучу потоков на покупку в одно время.
Тоже самое и с max_count.
Тоже самое и с

$user->money = $user->money - $item_price;

Они все - небезопасные операции.
Решение проблемы - индивидуальное для каждого проекта.

Изменено covobo (30.03.2018 12:14:07)

Не в сети

#3 30.03.2018 16:25:20

Re: Безопасность кода

Огромное спасибо. Действительно возможен дюп. А может быть подскажите как я могу (через какие инструменты) отправить сразу 10 запросов и купить 10 предметов по цене 1?

Не в сети

#4 30.03.2018 16:59:31

Re: Безопасность кода

Огромное спасибо. Действительно возможен дюп. А может быть подскажите как я могу (через какие инструменты) отправить сразу 10 запросов и купить 10 предметов по цене 1?

Огромное количество самописных магазинов подвержены этой уязвимостью. Расчет на то, что их магазин (магазин с таким багом) никому не нужен smile и на ручную модерацию.

Надо одновременно запустить кучу запросов на бэкенд. Думаю инструментов в публичном доступе много. Подсказать не смогу.
Может быть тот-же Postman так умеет.

Не в сети

#5 30.03.2018 20:14:51

Re: Безопасность кода

Решение проблемы - индивидуальное для каждого проекта.

Решение вполне универсальное, только детали запроса несколько зависят от ситуации. Об этом на форуме уже не раз поднимали темы. Пример, адаптированный отсюда:

UPDATE users
   SET money = @money := money - :money
 WHERE id = :id AND money >= :money

Однако само по себе списание с денег с баланса смысла обычно не имеет, то все это дело (списание, создание заказа и прочее) заворачивается в транзакцию. Если UPDATE выше вернул 0 затронутых строк - значит, баланс пользователя недостаточен. И это 100% потокобезопасно, т.к. БД не даст выполнить запрос (или транзакцию), который затрагивает значение, которое уже меняется кем-то параллельно.

SELECT @money; после запроса позволит получить актуальный баланс, тоже потокобезопасно.

А может быть подскажите как я могу (через какие инструменты) отправить сразу 10 запросов и купить 10 предметов по цене 1?

#!/bin/bash
for i in `seq 1 1000`; do
  curl localhost/buy/... &
done

Можно сделать HTML-страницу на том же домене и вставить туда сотню iframe, или сделать сотню запросов через XHR.

Не в сети

#6 30.03.2018 21:30:03

Re: Безопасность кода

Решение вполне универсальное, только детали запроса несколько зависят от ситуации.

Могу поспорить, что все сильно зависит от общих подходов в проекте.
Как минимум - вы показали вариант для SQL баз данных. Существуют ведь и другие?
Более того, в моем случае - нагрузки настолько высокие, что если я буду на каждое списывание выполнять запрос

UPDATE users
   SET money = @money := money - :money
 WHERE id = :id AND money >= :money

То я получу очень большое проседание производительности БД из-за локов (у меня 41k rps, в основном апдейты баланса на небольшой набор кортежей).
Поэтому ваше решение - не универсальное.

P.S. @Proger_XP вы часто "врываетесь" в топик последним, цитируете кучу сообщений, поправляете их (хотя все сообщения корректные и если в чем-то некорректные - то лишь ради упрощения для читателей), тем самым жестко нарушаете баланс "правило последнего впечатления" (остается негативное впечатления о всех предыдущих комментаторов). Лично мне это доставляет негативные эмоции, ибо вместо свободного общения, я жду, когда же придет Proger_XP и перекрестит здесь все. Спасибо.

Изменено covobo (30.03.2018 21:31:16)

Не в сети

#7 30.03.2018 21:54:48

Re: Безопасность кода

Как минимум - вы показали вариант для SQL баз данных. Существуют ведь и другие?

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

(у меня 41k rps, в основном апдейты баланса на небольшой набор кортежей).

Сколько дублирующихся строк (пользователей) из этих 41к? У вас биржа, где пользователи постоянно проводят транзакции? В этой теме речь идет об интернет-магазине, и я не представляю, какой из них может выдать хотя бы с десяток транзакций (покупок и пополнений) на одного и того же пользователя одновременно. Большая часть приложений именно такие.

тем самым жестко нарушаете баланс "правило последнего впечатления" (остается негативное впечатления о всех предыдущих комментаторов).

Серьезно, все обсуждения на форуме следует вести только в строго политкорректном, положительном ключе?

Если вы считаете, что кто-то выразился слишком категорично - так поправляйте, восстанавливайте "правило последнего впечатления". Критика - это нормально. В споре рождается истина, и пока нет перехода на личности - это тоже нормально.

Не в сети

#8 30.03.2018 22:03:02

Re: Безопасность кода

Вы назвали решение - универсальным. Я привер пример "Вот же, смотрите, не универсальное".

В этой теме речь идет об интернет-магазине

Выходит то решение не универсальное. Верно?
А специфично для интернет-магазинов, маленького и среднего оборота.

Если вы считаете, что кто-то выразился слишком категорично - так поправляйте, восстанавливайте "правило последнего впечатления". Критика - это нормально. В споре рождается истина, и пока нет перехода на личности - это тоже нормально.

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

Но, если я буду так реагировать на каждое сообщение, в котором меня некорректно поправят, то вместо форума про laravel - появится флудилка.
В большинстве случаев - мне все равно, но, вы - 1) администратор, 2) делаете это чаще всех.

Изменено covobo (30.03.2018 22:19:23)

Не в сети

#9 30.03.2018 22:35:46

Re: Безопасность кода

Выходит то решение не универсальное. Верно?

Верно. Я выразился слишком категорично.

А специфично для интернет-магазинов, маленького и среднего оборота.

...то есть для 9 из 10 от общего числа таких сайтов. Согласен, это не 100% универсальное решение, но я это говорил в ответ на "Решение проблемы - индивидуальное для каждого проекта", то есть для 90% условных интернет-магазинов это решение работает. Как минимум, я хотел донести, что в большинстве случаев не нужно изобретать велосипед.

Впредь буду выражаться точнее.

Не в сети

#10 01.04.2018 01:20:46

Re: Безопасность кода

А если поставить google recaptcha на покупку каждого предмета? Прокатит?

Не в сети

#11 01.04.2018 14:44:25

Re: Безопасность кода

А если поставить google recaptcha на покупку каждого предмета? Прокатит?

Это не решение проблемы, а уменьшение вероятности.
Proger_XP показал достаточное и простое решение проблемы, если оно тебя не устраивает - то остальное просто игра в "верю, не верю".

Рекаптча поможет, косвенно.

Изменено covobo (01.04.2018 15:13:27)

Не в сети

#12 01.04.2018 14:59:53

Re: Безопасность кода

Всё отлично. Вы мне вдвоём очень помогли, еще раз благодарю. Буду реализовывать способ от Proger_XP.

Не в сети

Подвал раздела