kaysoft - Hardcoded `deadline` for the `openPosition` params in the _placePerpOrder function

[github-actions[bot]]

kaysoft

medium

Hardcoded deadline for the openPosition params in the _placePerpOrder function

kaysoft

Medium

Summary

The deadline param for the openPosition is hardcoded to block.timestamp in the _placePerpOrder function.

Vulnerability Detail

The idea of the deadline is to allow the client to pass the deadline which is the time specified in milliseconds that have passed since the beginning of the Unix epoch plus allowed extra time (e.g 20minutes) so that the transaction fails whenever block.timestamp is more than the deadline. There is a check for require(deadline <= block.timestamp) to validate deadline. Since the deadline is hardcoded as block.timestamp , the deadline will be irrelevant since block.timestamp <= block.timestamp will always be true anytime the transaction is mined

Impact

Since the deadline is hardcoded as block.timestamp , the deadline will be irrelevant since block.timestamp <= block.timestamp will always be true anytime the transaction is mined.

Code Snippet

see: PerpDepository.sol#L362

The deadline is hardcoded to block.timestamp which will make the deadline parameter irrelevant. Allow the deadline to be passed as input parameter to the _placePerpOrder function.

function _placePerpOrder(
        uint256 amount,
        bool isShort,
        bool amountIsInput,
        uint160 sqrtPriceLimit
    ) private returns (uint256, uint256) {
        uint256 upperBound = 0; // 0 = no limit, limit set by sqrtPriceLimit

        IClearingHouse.OpenPositionParams memory params = IClearingHouse
            .OpenPositionParams({
                baseToken: market,
                isBaseToQuote: isShort, // true for short
                isExactInput: amountIsInput, // we specify exact input amount
                amount: amount, // collateral amount - fees
                oppositeAmountBound: upperBound, // output upper bound
                // solhint-disable-next-line not-rely-on-time
@>                deadline: block.timestamp,
                sqrtPriceLimitX96: sqrtPriceLimit, // max slippage
                referralCode: 0x0
            });

        (uint256 baseAmount, uint256 quoteAmount) = clearingHouse.openPosition(
@>            params
        );
        uint256 feeAmount = _calculatePerpOrderFeeAmount(quoteAmount);
        totalFeesPaid += feeAmount;

        emit PositionOpened(isShort, amount, amountIsInput, sqrtPriceLimit);
        return (baseAmount, quoteAmount);
    }

Tool used

Manual Review

Recommendation

Consider accepting the deadline as parameter to the _placePerpOrder function like below

function _placePerpOrder(
        uint256 amount,
        bool isShort,
        bool amountIsInput,
        uint160 sqrtPriceLimit,
+        uint256 deadline,
    ) private returns (uint256, uint256) {
        uint256 upperBound = 0; // 0 = no limit, limit set by sqrtPriceLimit

        IClearingHouse.OpenPositionParams memory params = IClearingHouse
            .OpenPositionParams({
                baseToken: market,
                isBaseToQuote: isShort, // true for short
                isExactInput: amountIsInput, // we specify exact input amount
                amount: amount, // collateral amount - fees
                oppositeAmountBound: upperBound, // output upper bound
                // solhint-disable-next-line not-rely-on-time
+                deadline: _deadline,
                sqrtPriceLimitX96: sqrtPriceLimit, // max slippage
                referralCode: 0x0
            });

        (uint256 baseAmount, uint256 quoteAmount) = clearingHouse.openPosition(
            params
        );
        uint256 feeAmount = _calculatePerpOrderFeeAmount(quoteAmount);
        totalFeesPaid += feeAmount;

        emit PositionOpened(isShort, amount, amountIsInput, sqrtPriceLimit);
        return (baseAmount, quoteAmount);
    }