code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/base/TalosBaseStrategy.sol#L208

Vulnerability details

Impact

Miner can potentially hold the transaction which results in loss of funds for users.

Proof of Concept

File: TalosBaseStrategy.sol

    (liquidityDifference, amount0, amount1) = nonfungiblePositionManager.increaseLiquidity(
        INonfungiblePositionManager.IncreaseLiquidityParams({
            tokenId: _tokenId,
            amount0Desired: amount0Desired,
            amount1Desired: amount1Desired,
            amount0Min: 0,     
            amount1Min: 0,
 @->        deadline: block.timestamp        
        })
    );

Link to code

AMMs provide their users with an option to limit the execution of their pending actions, such as swaps or adding and removing liquidity. The most common solution is to include a deadline timestamp as a parameter (for example in Uniswap V2 and Uniswap V3). If such an option is not present, users can unknowingly perform bad trades as interaction with the pools is very time sensitive, if transaction gets hold for sometime than it can lead to bad trades unknowingly by the Users and can potentially result in loss of funds.

All the functions in the codebase that interact with uniswap pools have a deadline parameter passed as block.timestamp, which means that whenever the miner decides to include the transaction in a block, it will be valid at that time, since block.timestamp will be the current timestamp at the time of mining. but here, there is a possibility of a malicious miner to hold the transaction how whatever amount he/she wants.

This is tbe reason, instead of block.timestamp, an extra arguments should be passed with the functions and user should be allowed to pass a valid deadline for it so that in case a malicious miner tries to hold it, then the transaction reverts as per the deadline specified by the user. Having slippage is an issue for any AMMs but having outdated Slippage protection can also be a dangerous attack vector.

Link to a similar issue reported in Paraspace Audit by Code4rena.

Tools Used

VS Code

Recommended Mitigation Steps

Add deadline arguments in all functions that interact with uniswap pools, and pass it along to these calls.

Assessed type

Uniswap

c4-judge commented 1 year ago

trust1995 marked the issue as duplicate of #171