Skip to content

Latest commit

 

History

History
96 lines (73 loc) · 4.72 KB

File metadata and controls

96 lines (73 loc) · 4.72 KB

Cheerful Taffy Dolphin

High

Missing Execution Rights Validation in _cancelOrder() Enables Front-Running of Signed Cancel Orders and Market Manipulation

Summary

The protocol implements two cancellation paths: direct cancellation and signature-based cancellation, utilizing EIP712 signatures for off-chain authorization. The Manager contract coordinates with an OrderVerifier for signature validation and a MarketFactory for operator permissions. A security review of this system reveals a significant authorization vulnerability in the order cancellation flow.

The issue stems from the dual authorization paths in the order cancellation system. In the Manager contract, direct cancellations flow through:

https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/main/perennial-v2/packages/periphery/contracts/TriggerOrders/Manager.sol#L112

function cancelOrder(IMarket market, uint256 orderId) external {
    _cancelOrder(market, msg.sender, orderId);
}

This path implicitly ensures authorization as the account parameter passed to _cancelOrder is msg.sender. However, in the signature-based path:

function cancelOrderWithSignature(CancelOrderAction calldata request, bytes calldata signature)
    external 
    keepAction(request.action, abi.encode(request, signature))
{
    verifier.verifyCancelOrder(request, signature);
    _cancelOrder(request.action.market, request.action.common.account, request.action.orderId);
}

it delegates authorization to the OrderVerifier contract, which validates through EIP712 signatures:

function _verifySignature(Action calldata action, bytes32 hash, bytes calldata signature) internal view {
    if (!SignatureChecker.isValidSignatureNow(
        action.common.signer,
        _hashTypedDataV4(hash),
        signature
    )) revert VerifierInvalidSignerError();
}

The authorization check in OrderVerifier only validates that the signer is authorized:

function _authorized(address account, address signer) internal view override returns (bool) {
    return super._authorized(account, signer) || marketFactory.signers(account, signer);
}

The critical vulnerability lies in _cancelOrder lacking execution authorization:

function _cancelOrder(IMarket market, address account, uint256 orderId) private {
    TriggerOrder memory order = _orders[market][account][orderId].read();
    if (order.isEmpty() || order.isSpent) revert ManagerCannotCancelError();
    order.isSpent = true;
    _orders[market][account][orderId].store(order);
    emit TriggerOrderCancelled(market, account, orderId);
}

This creates a security gap where anyone can execute a signed cancellation order. While the signature proves intent from an authorized signer, there's no validation of the executing party (msg.sender). This enables front-running attacks where malicious actors could execute signed cancellations before the intended executor.

Impact

The authorization vulnerability in _cancelOrder has severe implications for the trigger order system's integrity. Any party can execute a signed cancel order without execution rights, enabling front-running of cancellations in volatile market conditions. Adversaries could manipulate the timing of order cancellations to their advantage, particularly damaging in a DeFi context where order execution sequence directly impacts profit/loss outcomes. Given that the Manager contract handles trigger orders for positions in trading markets, unauthorized control over cancellation timing could be exploited to manipulate market positions and force unfavorable execution conditions.

Fix

The fix requires adding operator validation in _cancelOrder:

function _cancelOrder(IMarket market, address account, uint256 orderId) private {
    if (msg.sender != account && !marketFactory.operators(account, msg.sender)) 
        revert ManagerNotOperatorError();
        
    TriggerOrder memory order = _orders[market][account][orderId].read();
    if (order.isEmpty() || order.isSpent) revert ManagerCannotCancelError();
    order.isSpent = true;
    _orders[market][account][orderId].store(order);
    emit TriggerOrderCancelled(market, account, orderId);
}

This aligns with the contract's existing operator check pattern, seen in the claim function:

modifier onlyOperator(address account, address operator) {
    if (account != operator && !marketFactory.operators(account, operator)) 
        revert ManagerNotOperatorError();
    _;
}

The vulnerability has significant implications for market operations, particularly during volatile conditions where order cancellation timing is critical. The fix establishes proper dual-layer authorization: signature validation for intent and operator validation for execution rights.