howlbot-integration[bot] commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-06-vultisig/blob/main/src/ILOPool.sol#L242-L248 https://github.com/code-423n4/2024-06-vultisig/blob/main/src/ILOPool.sol#L257-L260

Vulnerability details

Impact

When a user claims their fees from ILOPool.claim, they receive their accrued fees up to that point. However, the function then attempts to send the performance fee to feeTaker, performing a collect on the Uniswap position with the amount type(uint128).max, which withdraws all fees from all users. This excess is then sent to feeTaker, causing potential DoS and future payment issues.

Proof of Concept

ILOPool.sol:

(uint128 amountCollected0, uint128 amountCollected1) = pool.collect(
            address(this),
            TICK_LOWER,
            TICK_UPPER,
            type(uint128).max,
            type(uint128).max
        );

...

        address feeTaker = IILOManager(MANAGER).FEE_TAKER();
        // transfer fee to fee taker
        TransferHelper.safeTransfer(_cachedPoolKey.token0, feeTaker, amountCollected0-amount0);
        TransferHelper.safeTransfer(_cachedPoolKey.token1, feeTaker, amountCollected1-amount1);

amountCollected{0,1} represent the accumulated fees from all users, so feeTaker is receiving almost all the benefits.

Tools Used

Foundry

Recommended Mitigation Steps

The function should send to feeTaker only the fees obtained relative to what the user claimed.

Assessed type

Uniswap

c4-judge commented 4 months ago

alex-ppg marked the issue as duplicate of #43