0xloophole - Inadequate Slippage Protection in Order Execution

[sherlock-admin3]

0xloophole

Medium

Inadequate Slippage Protection in Order Execution

Summary

The MultiInvoker contract manages various operations in the Perennial protocol, including the execution of trigger orders.

Details

The issue is in the _executeOrder function. While it does use trigger conditions to determine when to execute an order, it doesn't implement a mechanism to protect against slippage between the time an order is placed and when it's executed.

Code Snippet

function _executeOrder(address account, IMarket market, uint256 nonce) internal {
    if (!canExecuteOrder(account, market, nonce)) revert MultiInvokerCantExecuteError();

    TriggerOrder memory order = orders(account, market, nonce);

    _handleKeeperFee(
        KeepConfig(
            UFixed18Lib.ZERO,
            keepBufferBase,
            UFixed18Lib.ZERO,
            keepBufferCalldata
        ),
        0,
        msg.data[0:0],
        0,
        abi.encode(account, market, order.fee)
    );

    _marketSettle(market, account);

    Order memory pending = market.pendings(account);
    Position memory currentPosition = market.positions(account);
    currentPosition.update(pending);

    Fixed6 collateral = order.execute(currentPosition);

    _update(
        account,
        market,
        currentPosition.maker,
        currentPosition.long,
        currentPosition.short,
        collateral,
        true,
        order.interfaceFee1,
        order.interfaceFee2
    );

    delete _orders[account][market][nonce];
    emit OrderExecuted(account, market, nonce);
}

Impact

1. Unexpected Execution Prices: Orders may be executed at prices significantly different from what the user anticipated when placing the order.
2. Potential for Large Losses: In volatile market conditions, the lack of slippage protection could result in substantial losses for users.
3. Vulnerability to Market Manipulation: Malicious actors could potentially manipulate market conditions to trigger unfavorable order executions.

Scenario

Bob places a trigger order to go long when the price reaches a certain level. The price reaches this level and starts to rapidly increase. Bob's order is executed, but at a much higher price than he intended, resulting in a significant loss.

Fix

Implement a maximum slippage tolerance in the TriggerOrder structure and check it during execution:

struct TriggerOrder {
    // ... existing fields ...
    UFixed6 maxSlippage; // New field for maximum allowed slippage
}

function _executeOrder(address account, IMarket market, uint256 nonce) internal {
    // ... existing checks ...

    TriggerOrder memory order = orders(account, market, nonce);
    
    // Get the current market price
    UFixed6 currentPrice = market.getPrice();

    // Calculate the maximum acceptable price based on the trigger price and max slippage
    UFixed6 maxAcceptablePrice = order.triggerPrice.mul(UFixed6Lib.ONE.add(order.maxSlippage));

    // Check if the current price is within the acceptable range
    require(currentPrice.lte(maxAcceptablePrice), "Slippage exceeded");

    // ... rest of the function ...
}