sherlock-audit / 2023-11-olympus-judging

9 stars 7 forks source link

hash - Bunni liquidityReserves doesn't count owed fees #205

Closed sherlock-admin closed 6 months ago

sherlock-admin commented 6 months ago

hash

medium

Bunni liquidityReserves doesn't count owed fees

Summary

owed fees is not included in Bunni's protocol owned liquidity reserves

Vulnerability Detail

The getProtocolOwnedLiquidityReserves function returns the (liquidity reserves + uncollected fees)

        function getProtocolOwnedLiquidityReserves()
        external
        view
        override
        returns (SPPLYv1.Reserves[] memory)
    {
        .....

        for (uint256 i; i < len; ) {
            TokenData storage tokenData = bunniTokens[i];
            BunniToken token = tokenData.token;
            BunniLens lens = tokenData.lens;
            BunniKey memory key = _getBunniKey(token);
            (
                address token0,
                address token1,
                uint256 reserve0,
                uint256 reserve1
            ) = _getReservesWithFees(key, lens);
    function _getReservesWithFees(
        BunniKey memory key_,
        BunniLens lens_
    ) internal view returns (address, address, uint256, uint256) {
        (uint112 reserve0, uint112 reserve1) = lens_.getReserves(key_);
        (uint256 fee0, uint256 fee1) = lens_.getUncollectedFees(key_);

        return (key_.pool.token0(), key_.pool.token1(), reserve0 + fee0, reserve1 + fee1);
    }

But since Bunnihub has a function to account the owed fees, it is possible for token amounts to be present as positions owed token amounts. This amount will not be reported by the _getReservesWithFees function.

    function updateSwapFees(
        BunniKey calldata key
    ) external virtual override returns (uint256 swapFee0, uint256 swapFee1) {
        key.pool.burn(key.tickLower, key.tickUpper, 0);
        (, , , uint128 cachedFeesOwed0, uint128 cachedFeesOwed1) = key.pool.positions(
            keccak256(abi.encodePacked(address(this), key.tickLower, key.tickUpper))
        );

        return (cachedFeesOwed0, cachedFeesOwed1);
    }

Impact

Underreporting of the total token amounts available for a bunni token.

Code Snippet

https://github.com/sherlock-audit/2023-11-olympus/blob/9c8df76dc9820b4c6605d2e1e6d87dcfa9e50070/bophades/src/modules/SPPLY/submodules/BunniSupply.sol#L417-L425

Tool used

Manual Review

Recommendation

Include the owed token amounts

Duplicate of #49

sherlock-admin2 commented 6 months ago

1 comment(s) were left on this issue during the judging contest.

nirohgo commented:

Not a real issue. getUncollectedFees calculates owed fees without updating (changing state) whereas updateSwapFees causes univ3 to update the feeGrowthInside01Last values and move the owed fees to a separate accounting. In any case the resulting fees are the same.