sherlock-audit / 2023-11-olympus-judging

9 stars 8 forks source link

bin2chen - getWeightedPoolTokenPrice() wrongly assumes that all of the weighted pools uses totalSupply #145

Closed sherlock-admin closed 9 months ago

sherlock-admin commented 9 months ago

bin2chen

medium

getWeightedPoolTokenPrice() wrongly assumes that all of the weighted pools uses totalSupply

Summary

BalancerPoolTokenPrice.sol which is used specifically for weighted balancer pools wrongly uses totalSupply all the time to get the total supply of the pool, but this would not be true for newer weighted pools.

Vulnerability Detail

copy from: https://github.com/sherlock-audit/2023-10-notional-judging/issues/36

Balancer pools have different methods to get their total supply of minted LP tokens, which is also specified in the docs here https://docs.balancer.fi/concepts/advanced/valuing-bpt/valuing-bpt.html#getting-bpt-supply . The docs specifies the fact that totalSupply is only used for older stable and weighted pools, and should not be used without checking it first, since the newer pools have pre-minted BPT and getActualSupply should be used in that case. Most of the time, the assumption would be that only the new composable stable pools uses the getActualSupply, but that is not the case, since even the newer weighted pools have and uses the getActualSupply. To give you few examples of newer weighted pools that uses getActualSupply

https://etherscan.io/address/0x9f9d900462492d4c21e9523ca95a7cd86142f298 https://etherscan.io/address/0x3ff3a210e57cfe679d9ad1e9ba6453a716c56a2e https://etherscan.io/address/0xcf7b51ce5755513d4be016b0e28d6edeffa1d52a Because of that, the interaction with newer weighted pools and also the future weighted pools would not be accurate since the wrong total supply is used and calculations like getWeightedPoolTokenPrice() would be inaccurate

Note: AuraBalancerSupply.sol has same problem

Impact

error totalSupply, getWeightedPoolTokenPrice() will be inaccurate

Code Snippet

https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/modules/PRICE/submodules/feeds/BalancerPoolTokenPrice.sol#L402

Tool used

Manual Review

Recommendation

    function getTotalPoolSupply(address pool) internal view override returns (uint256) {
        // As of this writing the documentation linked above appears inaccurate and the
        // pre-minted total supply returned by a pool is not a fixed constant. Therefore,
        // we use a try / catch here instead.
        try IComposablePool(address(pool)).getActualSupply() returns (uint256 totalSupply) {
            return totalSupply;
        } catch {
            return pool.totalSupply();
        }
    }

Duplicate of #155