sherlock-audit / 2023-10-notional-judging

5 stars 5 forks source link

mstpr-brainbot - Balancer composable stable pools spot price calculation is wrong #91

Closed sherlock-admin2 closed 11 months ago

sherlock-admin2 commented 11 months ago

mstpr-brainbot

high

Balancer composable stable pools spot price calculation is wrong

Summary

When computing the spot price within Balancer's composable stable pools, the vault initially calculates the price using scaled factors but subsequently removes them, which is incorrect. Unlike in weighted pools, scale factors play a role in the mathematics of composable stable pools. Furthermore, Chainlink prices also take these scale factors into account. For instance, in the stETH Chainlink oracle, Chainlink initially queries the stETH to determine the amount of underlying ETH it represents, subsequently using the ETH/USD price for conversion. In Balancer cps pools, these scale factors are utilized during swaps, joins, and exits. Therefore, omitting the scaled factors from the final spot prices leads to incorrect results.

Vulnerability Detail

In Balancer, scaled factors can take on two different value types depending on the pool type.

For weighted pools, the scale factor represents the decimal precision required for the asset to achieve 18 decimals. Rates from rate providers are solely used for yield calculation and not for pool math.

On the other hand, in Composable Stable Pools (CPS), the scale factor encompasses both the decimal precision required for the asset to reach 18 decimals and the rate providers' rates. Hence, for these pools, the scale factor is computed as (10*(18-asset) rate).

For more info on this topic please refer to the official balancer documentation: https://docs.balancer.fi/reference/contracts/rate-providers.html#application-of-rate-providers-by-pool-type

Now, let's see how the CPS spot prices are calculated in vault here the related snippet:

function _calculateStableMathSpotPrice(
        uint256 ampParam,
        uint256[] memory scalingFactors,
        uint256[] memory balances,
        uint256 scaledPrimary,
        uint256 primaryIndex,
        uint256 index2
    ) internal pure returns (uint256 spotPrice) {
        // Apply scale factors
        uint256 secondary = balances[index2] * scalingFactors[index2] / BALANCER_PRECISION;

        uint256 invariant = StableMath._calculateInvariant(
            ampParam, StableMath._balances(scaledPrimary, secondary), true // round up
        );

        spotPrice = StableMath._calcSpotPrice(ampParam, invariant, scaledPrimary, secondary);

        // Remove scaling factors from spot price
        spotPrice = spotPrice * scalingFactors[primaryIndex] / scalingFactors[index2];
    }

In the above code, both balances are multiplied by the scaled factors before being utilized in the _calcSpotPrice function, which is accurate. However, the final line, which involves removing the scaling factors, could lead to inaccuracies. This is because CPS pools utilize the scaled prices within the mathematical operations. Removing these scaled prices for comparison with Chainlink prices would yield incorrect results.

For example, consider an rETH-ETH CPS pool in Balancer, where the pool holds 48% rETH and 52% ETH. In a standard pool, this distribution would represent 48 X tokens and 52 Y tokens. However, in a CPS pool with rate providers, let's assume the following:

rETH amount * scaledFactor = 48 ETH amount = 52

Given that 1 rETH is equivalent to 1.02 ETH, the pool actually contains 47.05882353 rETH and 52 ETH. However, the vault assumes there are 48 rETH and 52 WETH. This discrepancy arises because the vault uses raw balances rather than factoring in the rate providers' rates.

Regarding how Chainlink calculates the rETH price, Chainlink also utilizes rate providers. It initially queries the rocket pool to determine how much ETH underlies 1 rETH (employing the same rate as Balancer). Then, the resulting ETH amount is multiplied by the USD value of ETH. Chainlink does not derive the value from market trade prices but rather employs the rate provider to obtain the underlying ETH and price it accordingly. However, in the Balancer pool, when calculating the spot price, the vault extracts the spot price and uses the raw balance, which is inaccurate

Example contract, check the scale factors, they are not only the decimal scales but also the underlying tokens chainlink rate. https://arbiscan.io/address/0x4a2f6ae7f3e5d715689530873ec35593dc28951b#readContract

Impact

Balancer CPS pools with rate providers (LST's etc) will misprice the LP token price. Mispricing the LP token can lead to discrepancies which can lead to uneven liqudiations or valuing a collateral over or under priced. Hence, I will label this as high.

Code Snippet

https://github.com/sherlock-audit/2023-10-notional/blob/7aadd254da5f645a7e1b718e7f9128f845e10f02/leveraged-vaults/contracts/vaults/balancer/BalancerSpotPrice.sol#L49-L97

Tool used

Manual Review

Recommendation

Do not remove the scale factors from the final spot price. Only adjust the decimals.

Duplicate of #79