Lines of code

https://github.com/code-423n4/2024-05-predy/blob/main/src/libraries/Trade.sol#L53-L60 https://github.com/code-423n4/2024-05-predy/blob/main/src/libraries/Trade.sol#L94 https://github.com/code-423n4/2024-05-predy/blob/main/src/base/SettlementCallbackLib.sol#L113-L120 https://github.com/code-423n4/2024-05-predy/blob/main/src/base/SettlementCallbackLib.sol#L160-L167 https://github.com/code-423n4/2024-05-predy/blob/main/src/settlements/UniswapSettlement.sol#L33-L35 https://github.com/code-423n4/2024-05-predy/blob/main/src/settlements/UniswapSettlement.sol#L49-L51

Vulnerability details

Impact

The protocol utilizes Uniswap's swapExactIn and swapExactOut for executing trades.

The problem is that the deadline is always set to block.timestamp, which completely defeats the purpose of having a deadline parameter.

This is a known issue, where a malicious user or MEV bot can execute the transaction at a later time that benefits them.

Due to a lack of deadline, these swaps may cause a loss for the protocol.

Proof of Concept

When trades are executed, a swap is initiated

Trade.sol#L53-L60

    SwapStableResult memory swapResult = swap(
        globalData,
        tradeParams.pairId,
        SwapStableResult(-tradeParams.tradeAmount, underlyingAmountForSqrt, realizedFee.feeAmountBase, 0),
        settlementData,
        tradeResult.sqrtPrice,
        tradeParams.vaultId
    );

The swap function will make a call to the settlement callback

Trade.sol#L94

globalData.callSettlementCallback(settlementData, totalBaseAmount);

During the callback, depending on if base tokens are being bought or sold, swapExactIn or swapExactOut is called

SettlementCallbackLib.sol#L113-L120

    uint256 quoteAmountFromUni = ISettlement(settlementParams.contractAddress).swapExactIn(
        quoteToken,
        baseToken,
        settlementParams.encodedData,
        sellAmount,
        settlementParams.maxQuoteAmount,
        address(this)
    );

SettlementCallbackLib.sol#L160-L167

    uint256 quoteAmountToUni = ISettlement(settlementParams.contractAddress).swapExactOut(
        quoteToken,
        baseToken,
        settlementParams.encodedData,
        buyAmount,
        settlementParams.maxQuoteAmount,
        address(predyPool)
    );

The Uniswap Settlement has block.timestamp always set for the deadline

UniswapSettlement.sol#L33-L35

    amountOut = _swapRouter.exactInput(
        ISwapRouter.ExactInputParams(data, recipient, block.timestamp, amountIn, amountOutMinimum)
    );

UniswapSettlement.sol#L49-L51

    amountIn = _swapRouter.exactOutput(
        ISwapRouter.ExactOutputParams(data, recipient, block.timestamp, amountOut, amountInMaximum)
    );

This is dangerous as the transaction can be executed at a more favorable time for MEV bots or a malicious user.

Tools Used

Manual Review

Recommended Mitigation Steps

Add a deadline parameter to the trade functions, so that when a swap is executed, it will incorporate that deadline.

Assessed type

Uniswap

code-423n4 / 2024-05-predy-findings

Using `block.timestamp` as deadline is dangerous #274