code-423n4 / 2024-03-revert-lend-findings

9 stars 8 forks source link

Dangerous use of deadline parameter in AutoCompound.sol #515

Closed c4-bot-8 closed 6 months ago

c4-bot-8 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/transformers/AutoCompound.sol#L163

Vulnerability details

Impact

Actions in the Uniswap NonfungiblePositionManager contract are protected by a deadline parameter to limit the execution of pending transactions. Functions that modify the liquidity of the pool check this parameter against the current block timestamp in order to discard expired actions.

User provided deadline arguments are utilized in transformers AutoRange.sol and LeverageTransformer.sol while interacting with the Uniswap NFT Position Manager. However, the execute() function in AutoCompound.sol uses block.timestamp as the deadline argument, which defeats the purpose of using a deadline. Using block.timestamp as the deadline is effectively a no-operation that has no effect nor protection. Since block.timestamp will take the timestamp value when the transaction gets mined, the check will end up comparing block.timestamp against itself.

Failure to provide a proper deadline value enables pending transactions to be maliciously executed at a later point in detriment of the submitter.

AutoCompound.sol #L161-165 :

(, state.compounded0, state.compounded1) = nonfungiblePositionManager.increaseLiquidity(
                    INonfungiblePositionManager.IncreaseLiquidityParams(
                        params.tokenId, state.maxAddAmount0, state.maxAddAmount1, 0, 0, block.timestamp
                    )
                );

Tools Used

Manual Review

Recommended Mitigation Steps

Add a deadline parameter to the ExecuteParams struct and forward it to the corresponding underlying call to the Uniswap NonfungiblePositionManager contract.

Assessed type

Uniswap

c4-pre-sort commented 6 months ago

0xEVom marked the issue as duplicate of #147

0xEVom commented 6 months ago

Only mentions execute()

c4-pre-sort commented 6 months ago

0xEVom marked the issue as sufficient quality report

c4-judge commented 6 months ago

jhsagd76 marked the issue as satisfactory