sherlock-audit / 2023-11-olympus-judging

9 stars 7 forks source link

cu5t0mPe0 - _getOhmReserves calculates the number of ohm incorrectly #121

Closed sherlock-admin closed 6 months ago

sherlock-admin commented 6 months ago

cu5t0mPe0

high

_getOhmReserves calculates the number of ohm incorrectly

Summary

When the _getOhmReserves function calculates the ohm in the uniswap pool, if the pool does not contain ohm, the function will return an incorrect ohm amount.

Vulnerability Detail

[BunniSupply.sol#_getOhmReserves](https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/modules/SPPLY/submodules/BunniSupply.sol#L399-L409)

    function _getOhmReserves(
        BunniKey memory key_,
        BunniLens lens_
    ) internal view returns (uint256) {
        (uint112 reserve0, uint112 reserve1) = lens_.getReserves(key_);
        if (key_.pool.token0() == ohm) { 
            return reserve0;
        } else {
            return reserve1;
        }
    }

When this function determines whether it is Ohm, that is, if token0 is not Ohm, then token1 will be recognized as Ohm.

cu5t0mpeo — Yesterday at 13:10
In addition, the pool in the BunniSupply contract is uniswap pool, right? If it is a uniswap pool, does it require that one of the tokens in the pool must be ohm?

Jem: It does not require one of the tokens to be OHM, but that is likely how it will be used.

However, according to the project's reply, the pool does not necessarily contain ohm token, so an incorrect result will be returned in the end.

Impact

Returns an incorrect ohm amount as a result, affecting function: getProtocolOwnedLiquidityOhm

Code Snippet

https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/modules/SPPLY/submodules/BunniSupply.sol#L399-L409

Tool used

Manual Review

Recommendation

Both token0 and token1 should determine whether they are ohm tokens.

0xJem commented 6 months ago

The intention of the system is to only have OHM pairs, but this is technically possible

nevillehuang commented 6 months ago

@0xJem the whole purpose of BunniSupply is to track ohm pairs which is clearly noted in this comment here so suggest to keep invalid since this would constitute future integrations

0xJem commented 6 months ago

https://github.com/OlympusDAO/bophades/pull/267

IAm0x52 commented 6 months ago

Fix looks good. Adds extra check.