sherlock-audit / 2023-06-real-wagmi-judging

3 stars 2 forks source link

0xpinky - Multipool.sol: No fee would be collected when calling the `function earn() external` #169

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

0xpinky

medium

Multipool.sol: No fee would be collected when calling the function earn() external

Summary

Since the inner function _earn takes the first argument 'lpAmount' as zero, inside the _withdraw there will not be any fee updates.

Vulnerability Detail

As the comments indicates , the fee would be collected and updated while calling the earn function.

 * @notice This function collects fees from the liquidity pool and updates the fee growth inside both Vaults per share
 *         based on the amount of fees collected. It then returns the updated fee growth values.
 * @dev When called, this function updates the fee growth inside each Vault according to the realised fees in the current block,
 *      adding them to the `feesGrowthInsideLastX128` struct member variable of the contract.
 */

But, the first argument would be key to decide the valid amount of lpAmount at Line505.

Since the lpAmount is set as zero, the calculation at above mentioned line would result the value of liquidityToWithdraw as zero.

So, there will not be any liquidity amount to burn and update the fee value in the following lines of codes.

            (, , , uint128 tokensOwed0Before, uint128 tokensOwed1Before) = IUniswapV3Pool(
                position.poolAddress
            ).positions(position.positionKey);

            (uint256 amount0, uint256 amount1) = IUniswapV3Pool(position.poolAddress).burn(
                position.lowerTick,
                position.upperTick,
                liquidityToWithdraw
            );

            withdrawnAmount0 += amount0;
            withdrawnAmount1 += amount1;

            (, , , uint128 tokensOwed0After, uint128 tokensOwed1After) = IUniswapV3Pool(
                position.poolAddress
            ).positions(position.positionKey);

            fee0 += (tokensOwed0After - amount0) - tokensOwed0Before;
            fee1 += (tokensOwed1After - amount1) - tokensOwed1Before; 

Impact

There will not be any fee updates which is mentioned as per the natsec comments. The dev intention is not fulfilled.

Code Snippet

https://github.com/sherlock-audit/2023-06-real-wagmi/blob/main/concentrator/contracts/Multipool.sol#L234-L243

https://github.com/sherlock-audit/2023-06-real-wagmi/blob/main/concentrator/contracts/Multipool.sol#L488-L543

Tool used

Manual Review

Recommendation

If fee want to be updated while calling the earn function, pass the first argument as 1 when calling _withdraw inside the _earn function. function _earn(uint256 _totalSupply) private { if (_totalSupply > 0) { _withdraw(0, _totalSupply); ---------------->> pass the first arg as 1 } }

Duplicate of #88