Skip to content

Race condition on discount usage_limit allows silent oversell on Black Friday #510

@mckenziearts

Description

@mckenziearts

Summary

CreateOrderFromCartAction creates the Order record before checking and incrementing the discount's total_use counter. Under concurrent checkout pressure (Black Friday, flash sale, viral coupon), the global usage_limit is silently exceeded: orders are committed with the discount fully applied to price_amount while the counter blocks at usage_limit. The merchant has no signal that an oversell happened.

A second related bug: usage_limit_per_user is effectively a no-op because the counter it relies on (DiscountDetail.total_use) is never incremented anywhere in the codebase.

Reproduction

  1. Create a discount with usage_limit = 1000 and is_active = true
  2. Have N concurrent customers (N > 1000) reach the checkout page with the coupon applied
  3. All N requests pass DiscountValidator (which reads total_use < usage_limit without locking)
  4. All N requests enter CreateOrderFromCartAction::execute() simultaneously
  5. Each request runs Order::create() first, succeeds, then attempts Discount::query()->where(...)->increment('total_use')
  6. Once total_use hits usage_limit, the conditional WHERE clause stops matching and increment() returns 0 — but the surrounding code does not check that return value
  7. Orders 1001..N exist in the database with the discount applied, while the counter remains stuck at 1000

Faulty code

packages/cart/src/Actions/CreateOrderFromCartAction.php (current 2.x):

public function execute(Cart $cart): Order
{
    return DB::transaction(function () use ($cart): Order {
        // ...
        $order = Order::query()->create([...]);     // committed first
        // ...

        if ($cart->coupon_code) {
            Discount::query()
                ->where('code', $cart->coupon_code)
                ->where(function ($query): void {
                    $query->whereNull('usage_limit')
                        ->orWhereColumn('total_use', '<', 'usage_limit');
                })
                ->increment('total_use');           // result ignored
        }

        // ...
    });
}

Per-user limit silent bypass

packages/cart/src/Discounts/DiscountValidator.php reads DiscountDetail.total_use to enforce usage_limit_per_user:

$userUses = $discount->items()
    ->where('condition', DiscountCondition::Eligibility)
    ->where('discountable_type', config('auth.providers.users.model'))
    ->where('discountable_id', $context->cart->customer_id)
    ->value('total_use') ?? 0;

if ($userUses > 0) {
    return new DiscountValidationResult(false, ...);
}

grep across the codebase confirms DiscountDetail.total_use is never incremented by any code path. The check therefore always sees 0 and validation passes regardless of how many times the customer has already redeemed the coupon. In addition, the DiscountDetail row only exists when eligibility = Customers, so for eligibility = Everyone the per-user limit cannot fire at all.

Impact

  • Severity: high — direct financial loss. Each over-redemption is a discount the merchant did not intend to grant.
  • Likelihood: high under marketing peaks (Black Friday, flash sales, time-bounded promo codes shared on social media).
  • Detection: silent. Counter shows usage_limit reached, but the actual number of discounted orders is greater. Discrepancy only surfaces in manual reconciliation.
  • Cross-tenant exposure: none (Shopper is single-tenant per install), but every install running discounts is exposed.

Expected behaviour

  1. The discount usage slot must be reserved atomically before Order::create() runs, inside the same DB::transaction.
  2. The atomic UPDATE must use the conditional WHERE total_use < usage_limit clause and check the affected rows count. If 0, throw an exception so the surrounding transaction rolls back and no order is committed.
  3. usage_limit_per_user must be enforced by counting actual prior orders for the same customer_id and discount_id, not by reading a counter that nothing increments.

Suggested fix

Reserve the usage slot before creating the order, throw a dedicated exception when the global or per-user limit was exhausted between cart validation and commit:

private function reserveDiscount(Cart $cart): ?Discount
{
    if (! $cart->coupon_code) {
        return null;
    }

    $discount = Discount::query()
        ->where('code', $cart->coupon_code)
        ->lockForUpdate()
        ->first();

    if ($discount === null) {
        return null;
    }

    if ($discount->usage_limit_per_user && $cart->customer_id !== null) {
        $alreadyRedeemed = Order::query()
            ->where('discount_id', $discount->id)
            ->where('customer_id', $cart->customer_id)
            ->exists();

        if ($alreadyRedeemed) {
            throw DiscountLimitReachedException::perUser($discount->code);
        }
    }

    $affected = Discount::query()
        ->whereKey($discount->id)
        ->where(function ($query): void {
            $query->whereNull('usage_limit')
                ->orWhereColumn('total_use', '<', 'usage_limit');
        })
        ->increment('total_use');

    if ($affected === 0) {
        throw DiscountLimitReachedException::global($discount->code);
    }

    return $discount->refresh();
}

This is a compare-and-swap pattern — same approach Stripe uses for coupon.times_redeemed and MedusaJS for PromotionService.applyPromotion.

The per-user check requires a foreign key from orders to discounts (orders.discount_id) so the count can be computed reliably across history. A snapshot of the redeemed code/value/currency on the order is also recommended for resilience against later discount edits or deletions (Shopify keeps both an FK and a string snapshot for the same reason).

References

  • packages/cart/src/Actions/CreateOrderFromCartAction.php — site of the race
  • packages/cart/src/Discounts/DiscountValidator.php — site of the per-user silent bypass
  • Stripe Coupons docs: idempotent redemption via server-side counter
  • MedusaJS PromotionService: UPDATE ... WHERE usage_count < usage_limit then throw on affected === 0

Acceptance criteria

  • Concurrent calls to CreateOrderFromCartAction::execute with the same coupon and a global usage_limit cannot produce more orders than usage_limit
  • When the limit is reached between cart validation and order commit, the action throws and no order row is inserted (transaction rollback)
  • usage_limit_per_user rejects a second redemption attempt by the same customer, regardless of eligibility mode
  • Test coverage: at minimum one test asserting no extra orders past the limit, and one test asserting per-user rejection on second use

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingconfirmedBug has been confirmedhigh priorityphpPull requests that update php codesecurity

    Type

    No fields configured for Bug.

    Projects

    Status
    Done

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions