shealtielanz - `AuraBalancerSupply.sol` wrongly assumes that all of the weighted pools uses `totalSupply`

[sherlock-admin2]

shealtielanz

medium

AuraBalancerSupply.sol wrongly assumes that all of the weighted pools uses totalSupply

Summary

AuraBalancerSupply.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

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 the last one also being on Aura finance. 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 in
_getReserves() https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/modules/SPPLY/submodules/AuraBalancerSupply.sol#L332C1-L347C81

    /// @notice         Get the reserves of a Balancer/Aura pool pair
    /// @dev            The calling function is responsible for protecting against re-entrancy.
    ///
    /// @param pool     Balancer/Aura pool pair
    /// @return         Reserves of the pool
    function _getReserves(Pool storage pool) internal view returns (SPPLYv1.Reserves memory) {
..SNIP..
@@>        uint256 balTotalSupply = pool.balancerPool.totalSupply();
        uint256[] memory balances = new uint256[](_vaultTokens.length);
        // Calculate the proportion of the pool balances owned by the polManager
        if (balTotalSupply != 0) {
            // Calculate the amount of OHM in the pool owned by the polManager
            // We have to iterate through the tokens array to find the index of OHM
            uint256 tokenLen = _vaultTokens.length;
            for (uint256 i; i < tokenLen; ) {
                uint256 balance = _vaultBalances[i];
                uint256 polBalance = (balance * balBalance) / balTotalSupply;

                balances[i] = polBalance;

                unchecked {
                    ++i;
                }
            }
        }

        SPPLYv1.Reserves memory reserves;
        reserves.source = address(pool.balancerPool);
        reserves.tokens = _vaultTokens;
        reserves.balances = balances;
        return reserves;
}

Impact

The calculation of reserves and the value of the LP tokens is very important for the protocol, and any miscalculations could hurt the protocol and the users a lot. Also according to Czar

I would like to draw Watsons' attention to the fact that despite any impact on a view function is a low severity finding per Sherlock rules, in this contest some view functions are to be used by the wider system (not necessarily in-scope), so if the system itself (not external codebases) will be impacted, the finding may be considered valid. Such a finding needs to contain an impact description that draws conclusions in the whole system (i.e. there needs to be a loss of funds/core functionality within the system)

Code Snippet

• https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/modules/SPPLY/submodules/AuraBalancerSupply.sol#L345

  Tool used

  Manual Review

Recommendation

Since the protocol would want to interact with multiple weighted pools, try to check first if the weighted pool you are interacting with is a newer one and uses the getActualSupply or if it is an older one and uses totalSupply, in that way the protocol could interact with multiple pools in the long run.

Duplicate of #155