sherlock-audit / 2022-09-notional-judging

4 stars 2 forks source link

GalloDaSballo - M-03 Incorrect "linear projection" for exponential math in calculating BPT value #129

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

GalloDaSballo

medium

M-03 Incorrect "linear projection" for exponential math in calculating BPT value

Summary

https://github.com/notional-finance/leveraged-vaults/blob/91b96a456a6ada5a7f73ecf420e4ac5bf6797b45/contracts/vaults/balancer/internal/pool/Boosted3TokenPoolUtils.sol#L203-L216

        // NOTE: For Boosted 3 token pools, the LP token (BPT) is just another
        // token in the pool. So, we first use _calcTokenOutGivenExactBptIn
        // to calculate the value of 1 BPT. Then, we scale it to the BPT
        // amount to get the value in terms of the primary currency.
        // Use virtual total supply and zero swap fees for joins
        primaryAmount = StableMath._calcTokenOutGivenExactBptIn({
            amp: oracleContext.ampParam, 
            balances: balances, 
            tokenIndex: 0, 
            bptAmountIn: BalancerConstants.BALANCER_PRECISION, // 1 BPT 
            bptTotalSupply: virtualSupply, 
            swapFeePercentage: 0, 
            currentInvariant: invariant
        });

Vulnerability Detail

BPT token value per fair math is going to be P t_n^wn / wn

Because of that, a linear approximation is not correct and instead price is dependent on size Because all tools are available, a direct quote should be preferred over retrieving 1.

Alternatively a full derivation for Balancer Pool Token Pricing is available here: https://twitter.com/0xa9a/status/1539227193808629761/photo/1

Impact

Because price derivation is linearly approximated, MEV is available by causing the strategy to miscalculate the assets it manages, extracting the difference

Code Snippet

Tool used

Manual Review

Recommendation

Use BPT derivation formula for the entire amount, do not use a linear approximation

jeffywu commented 1 year ago

@T-Woodward / @weitianjie2000, my understanding is that MEV for the boosted pool would be excessively expensive relative to potential profit unless the pool is at the boundaries.

Perhaps we should add some price check against the chainlink oracle to ensure that we are within some safe boundaries.

T-Woodward commented 1 year ago

The purpose of this function is to calculate the value of the BPTs without any slippage adjustments. The reason that this calculation is "wrong" is because of the slippage involved when you take out more than 1 BPT at a time. We account for slippage concerns in other ways (liquidation discounts, maximum leverage ratios, total vault capacities, etc).