sherlock-audit / 2023-11-olympus-judging

9 stars 7 forks source link

ast3ros - Incorrect calculation of pool token for the Balancer stable pool #192

Closed sherlock-admin2 closed 6 months ago

sherlock-admin2 commented 6 months ago

ast3ros

high

Incorrect calculation of pool token for the Balancer stable pool

Summary

The calculation method for the price of pool tokens in the Balancer stable pool is flawed, resulting in incorrect valuations.

Vulnerability Detail

In the getStablePoolTokenPrice function, the price of the pool token for a Balancer stable pool is calculated using the formula: minimumPrice * pool.getRate(). The minimumPrice is derived by comparing the price of each token in the pool.

    uint256 minimumPrice; // outputDecimals_
    {
        /**
         * The Balancer docs do not currently state this, but a historical version noted
         * that getRate() should be multiplied by the minimum price of the tokens in the
         * pool in order to get a valuation. This is the same approach as used by Curve stable pools.
         */
        for (uint256 i; i < len; i++) {
            address token = tokens[i];
            if (token == address(0)) revert Balancer_PoolTokenInvalid(poolId, i, token);

            /**
             * PRICE will revert if there is an issue resolving the price, or if it is 0.
             *
             * As the value of the pool token is reliant on the price of every underlying token,
             * the revert from PRICE is not caught.
             */
            (uint256 price_, ) = _PRICE().getPrice(token, PRICEv2.Variant.CURRENT); // outputDecimals_

            if (Using the incorrect poolSupply value leads to a miscalculation of the poolTokenPrice. The protocol will exchanging ohm for the assets at a wrong price. == 0) {
                minimumPrice = price_;
            } else if (price_ < minimumPrice) {
                minimumPrice = price_;
            }
        }
    }

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

However, this approach to calculating the minimum price is flawed. The constituent tokens in a stable pool may have different peg levels, and using the minimum formula without considering this variation can lead to significantly incorrect results.

    The introduction of `RateProviders` into the `StablePool` creates an obvious issue. Consider the computation of the
    minimum price (we use $P_i$ to denote the price of constituent $i$):

    $$ minPrice = min({P_0}, {P_1}, {P_2}) $$

    If the constituent tokens have `RateProviders`, then by definition they cannot be **like-kind assets**.
    Theoretically, they are all somehow pegged to the same "base asset", or else they wouldn't be included in
    a `StablePool`, but the nature of each peg can now differ. One token might be worth $1 while another is worth
    $1.50 and the third $100. If that's the case, then the minimum formula is not applicable and in fact returns a
    very wrong result.

https://github.com/balancer/docs/blob/663e2f4f2c3eee6f85805e102434629633af92a2/docs/concepts/advanced/valuing-bpt/bpt-as-collateral.md?plain=1#L44-L53

The correct approach to calculating the minimum price is to consider both the market price and the RateProvider price, normalizing them for a fair comparison:

    The constituent tokens have two different valuations: the market price (Chainlink oracles) and the pool price (from a RateProvider). The market price and the RateProvider price are usually close in value, so a division between these numbers tends to be close to 1, making the division a good candidate to normalize prices and find the minimum price. So, the minPrice (from the minPrice * pool.getRate() formula described above) would be calculated using

    $$ minPrice = min({P_{M_0} \over P_{RP_0}}, {P_{M_1} \over P_{RP_1}}, {P_{M_2} \over P_{RP_2}}, ...) $$

    Where:
    Pm:  is market price of constituent i;
    Prp: is the RateProvider price for constituent i. When no RateProvider is available, use 1e18.

https://github.com/balancer/docs/blob/663e2f4f2c3eee6f85805e102434629633af92a2/docs/concepts/advanced/valuing-bpt/bpt-as-collateral.md?plain=1#L67-L72

Impact

The current miscalculation of minimumPrice leads to an inaccurate valuation of poolTokenPrice, resulting in the protocol potentially exchanging OHM for assets at incorrect prices.

Code Snippet

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

Tool used

Manual review

Recommendation

Revise the calculation of the minimum price by incorporating both market prices and RateProvider prices. Example:


                (uint256 price_, ) = _PRICE().getPrice(token, PRICEv2.Variant.CURRENT); // outputDecimals_
+               price_ = price_ * 1e18 / pool.getTokenRate(token);
                if (minimumPrice == 0) {
                    minimumPrice = price_;
                } else if (price_ < minimumPrice) {
                    minimumPrice = price_;
                }

Duplicate of #176