Reserve Protocol - Invitational - ronnyx2017's results

Lines of code

https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/plugins/assets/cbeth/CBETHCollateral.sol#L67-L69 https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/plugins/assets/ankr/AnkrStakedEthCollateral.sol#L58-L61

Vulnerability details

The CBEthCollateral._underlyingRefPerTok() function just uses CBEth.exchangeRate() to get the ref/tok rate. The CBEth.exchangeRate() can only get the conversion rate from cbETH to staked ETH2 on the coinbase. However as the docs https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/2023-07-reserve/protocol/contracts/plugins/assets/cbeth/README.md the ref unit should be ETH. The staked ETH2 must take a few days to unstake, which leads to a premium between ETH and cbETH.

And the AnkrStakedEthCollateral and RethCollateral has the same problem. According to the ankr docs, unstake eth by Flash unstake have to pay a fee, 0.5% of the unstaked amount. https://www.ankr.com/docs/liquid-staking/eth/unstake/

Impact

The _underlyingRefPerTok will return a higher ref/tok rate than the truth. And the premium is positively correlated with the unstake delay of eth2. When the unstake queue suddenly increases, the attacker can uses cbeth to issue more rtokens. Even if the cbETH has defaulted, the CBEthCollateral will never mark the state as DISABLED because the CBEth.exchangeRate() is updated by coinbase manager and it only represents the cbETH / staked eth2 rate instead of the cbETH/ETH rate.

Proof of Concept

For example, Now it's about 17819370 block high on the mainnet, and the CBEth.exchangeRate()(https://etherscan.io/token/0xbe9895146f7af43049ca1c1ae358b0541ea49704#readProxyContract#F12) is 1.045264058480813188, but the chainlink price feed for cbETH/ETH(https://data.chain.link/ethereum/mainnet/crypto-eth/cbeth-eth) is 1.0438.

Tools Used

Manual review

Recommended Mitigation Steps

Use the cbETH/ETH oracle to get the cbETH/ETH rate.

Or, the ref unit for the collateral should be the staked eth2.

Assessed type

Context

Or, the ref unit for the collateral should be the staked eth2.

Lines of code

https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/plugins/assets/curve/CurveVolatileCollateral.sol#L32-L65

Vulnerability details

Impact

Attacker can make the CurveVolatileCollateral enter the status of IFFY/DISABLED . It will cause the basket to rebalance and sell off all the CurveVolatileCollateral.

Proof of Concept

The CurveVolatileCollateral overrides the _anyDepeggedInPool function to check if the distribution of capital is balanced. If the any part of underlying token exceeds the expected more than _defaultThreshold, return true, which means the volatile pool has been depeg:

uint192 expected = FIX_ONE.divu(nTokens); // {1}
for (uint8 i = 0; i < nTokens; i++) {
    uint192 observed = divuu(vals[i], valSum); // {1}
    if (observed > expected) {
        if (observed - expected > _defaultThreshold) return true;
    }
}

And the coll status will be updated in the super class CurveStableCollateral.refresh():

if (low == 0 || _anyDepeggedInPool() || _anyDepeggedOutsidePool()) {
    markStatus(CollateralStatus.IFFY);
}

The attack process is as follows:

1. Assumption: There is a CurveVolatileCollateral bases on a TriCrypto ETH/WBTC/USDT, and the vaule of them should be 1:1:1, and the _defaultThreshold of the CurveVolatileCollateral is 5%. And at first, there are 1000 USDT in the pool and the pool is balanced.

2. The attacker uses flash loan to deposit 500 USDT to the pool. Now, the USDT distribution is 1500/(1500+1000+1000) = 42.86% .

3. Attacker refresh the CurveVolatileCollateral. Because the USDT distribution - expected = 42.86% - 33.33% = 9.53% > 5% _defaultThreshold . So CurveVolatileCollateral will be marked as IFFY.

4. The attacker withdraw from the pool and repay the USDT.

5. Just wait delayUntilDefault, the collateral will be marked as defaulted by the alreadyDefaulted function.

    function alreadyDefaulted() internal view returns (bool) {
        return _whenDefault <= block.timestamp;
    }

Tools Used

Manual review

Recommended Mitigation Steps

I think the depegged status in the volatile pool may be unimportant. It will be temporary and have little impact on the price of outside lp tokens. After all, override the _anyDepeggedOutsidePool to check the lp price might be a good idea.

Assessed type

Context

Judge has assessed an item in Issue #26 as 2 risk. The relevant finding follows:

Curve Read-only Reentrancy can increase the price of some CurveStableCollateral

Lines of code

https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/plugins/assets/curve/CurveStableMetapoolCollateral.sol#L83-L86 https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/plugins/assets/curve/CurveStableCollateral.sol#L74-L98

Vulnerability details

The CurveStableMetapoolCollateral is intended for 2-fiattoken stable metapools. The metapoolToken coin0 is pairedToken and the coin1 is lpToken, e.g. 3CRV. And the config.chainlinkFeed should be set for paired token.

Impact

The CurveStableMetapoolCollateral.price() high price will be about FIX_MAX / metapoolToken.totalSupply() when the price oracle of pairedToken is timeout. It is significantly more than the actual price. It will lead to unexpected pricing in the rewards trade and rebalance auctions. Further more, I think an attacker can trigger this bug proactively by out of gas, which can bypass the empty error message check because of the different call stack depth. But I have not verified the idea due to lack of time. So the issue here only details the high price caused by external factor, for example oracle timeout. Hope to add it under this issue if I have any other progress. Thanks.

Proof of Concept

In the CurveStableMetapoolCollateral.tryPrice function, the pairedToken price is from tryPairedPrice function by the following codes:

uint192 lowPaired;
uint192 highPaired = FIX_MAX;
try this.tryPairedPrice() returns (uint192 lowPaired_, uint192 highPaired_) {
    lowPaired = lowPaired_;
    highPaired = highPaired_;
} catch {}

function tryPairedPrice() public view virtual returns (uint192 lowPaired, uint192 highPaired) {
    uint192 p = chainlinkFeed.price(oracleTimeout); // {UoA/pairedTok}
    uint192 delta = p.mul(oracleError, CEIL);
    return (p - delta, p + delta);
}

So if the chainlinkFeed is offline(oracle timeout), the tryPairedPrice will throw an error which is catched by the empty catch block, and the price of pairedToken will be (0, FIX_MAX).

And then the function _metapoolBalancesValue will use these pirces to get the total UoA of the metapool. The following codes are how it uses the price of pairedToken:

aumLow += lowPaired.mul(pairedBal, FLOOR);

// Add-in high part carefully
uint192 toAdd = highPaired.safeMul(pairedBal, CEIL);
if (aumHigh + uint256(toAdd) >= FIX_MAX) {
    aumHigh = FIX_MAX;
} else {
    aumHigh += toAdd;
}

The aumLow has already included the UoA of LpToken, so it is non-zero. And the highPaired price now is FIX_MAX, which will mul the paired token balance by Fixed.safeMul. We can find the Fixed lib has handled overflow safely:

function safeMul(
    uint192 a,
    uint192 b,
    RoundingMode rounding
) internal pure returns (uint192) {
    ...
    if (a == FIX_MAX || b == FIX_MAX) return FIX_MAX;

So the aumHigh from the _metapoolBalancesValue function will be FIX_MAX. The final prices are calculated by:

low = aumLow.div(supply, FLOOR);
high = aumHigh.div(supply, CEIL);

supply is the metapoolToken.totalSupply(). So if the supply is > 1 token, the Fixed.div won't revert. And the high price will be a huge but valid value < FIX_MAX.

Tools Used

Manual review

Recommended Mitigation Steps

Don't try catch the this.tryPairedPrice() in the CurveStableMetapoolCollateral.tryPrice, if it failed, just let the whole tryPrice function revert, the caller, for example refresh(), can catch the error.

Assessed type

Context

Lines of code

https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/plugins/assets/Asset.sol#L139-L145

Vulnerability details

When the tryPrice() function revert, for example oracle timeout, the Asset.lotPrice will use a decayed historical value:

uint48 delta = uint48(block.timestamp) - lastSave; // {s}
if (delta <= oracleTimeout) {
    lotLow = savedLowPrice;
    lotHigh = savedHighPrice;
} else if (delta >= oracleTimeout + priceTimeout) {
    return (0, 0); // no price after full timeout
} else {

And the delta time is from the last price saved time. If the delta time is greater than oracle timeout, historical price starts decaying.

But the last price might be saved at the last second of the last oracle timeout period. So the Asset.lotPrice will double the oracle timeout in the worst case.

Impact

The Asset.lotPrice will double the oracle timeout in the worst case. When the rewards need to be sold or basket is rebalancing, if the price oracle is offline temporarily, the Asset.lotPrice will use the last saved price in max two oracle timeout before the historical value starts to decay. It increases the sale/buy price of the asset.

Proof of Concept

The lastSave is updated in the refresh() function, and it's set to the current block.timestamp instead of the updateTime from the chainlink feed:

function refresh() public virtual override {
    try this.tryPrice() returns (uint192 low, uint192 high, uint192) {
        if (high < FIX_MAX) {
            savedLowPrice = low;
            savedHighPrice = high;
            lastSave = uint48(block.timestamp);

But in the OracleLib, the oracle time is checked for the delta time of block.timestamp - updateTime:

uint48 secondsSince = uint48(block.timestamp - updateTime);
if (secondsSince > timeout) revert StalePrice();

So if the last oracle feed updateTime is block.timestamp - priceTimeout, the timeout check will be passed and lastSave will be updated to block.timestamp. And the lotPrice will start to decay from lastSave + priceTimeout. However when it starts, it's been 2 * priceTimeout since the last oracle price update.

Tools Used

Manual review

Recommended Mitigation Steps

Starts lotPrice decay immediately or updated the lastSave to updateTime instead of block.timestamp.

Assessed type

Context

2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.

Lines of code

https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/plugins/assets/curve/CurveStableRTokenMetapoolCollateral.sol#L46-L54 https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/plugins/assets/curve/CurveStableCollateral.sol#L119-L121 https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/plugins/assets/curve/CurveStableMetapoolCollateral.sol#L122-L138

Vulnerability details

The CurveStableMetapoolCollateral is intended for 2-fiattoken stable metapools that involve RTokens, such as eUSD-fraxBP. The metapoolToken coin0 is pairedToken, which is also a RToken, and the coin1 is lpToken, e.g. 3CRV. And the CurveStableRTokenMetapoolCollateral.tryPairedPrice uses RTokenAsset.price() as the oracle to get the pairedToken price:

function tryPairedPrice()
    ...
    returns (uint192 lowPaired, uint192 highPaired)
{
    return pairedAssetRegistry.toAsset(pairedToken).price();
}

Impact

Users can't redeem from RToken when any underlying collateral of paired RToken's price oracle is offline(timeout). It can lead to a serious run/depeg on the RToken.

Proof of Concept

First I submitted another issue named "RTokenAsset price oracle can return a huge but valid high price when any underlying collateral's price oracle timeout". It's the premise for this issue. Because this issue is located in different collateral codes, I split them into two issues.

The conclusion from the pre issue:

If there is any underlying collateral's price oracle reverts, for example oracle timeout, the `RTokenAsset.price` will return a valid but untrue (low, high) price range, which can be described as `low = true_price * A1` and `high = FIX_MAX * A2`, A1 is `bh.quantity(oracle_revert_coll) / all quantity for a BU` and A2 is the `BasketRange.top / RToken totalSupply`.

Back to the CurveStableRTokenMetapoolCollateral. There are two cases that will revert in the super class CurveStableCollateral.refresh().

The CurveStableRTokenMetapoolCollateral.tryPairedPrice function gets low/high price from paired RTokenAsset.price(). So when any underlying collateral's price oracle of paired RTokenAsset reverts, the max high price will be FIX_MAX and the low price is non-zero.

1. If the high price is FIX_MAX, the assert for low price will revert:

if (high < FIX_MAX) {
    savedLowPrice = low;
    savedHighPrice = high;
    lastSave = uint48(block.timestamp);
} else {
    // must be unpriced
    // untested:
    //      validated in other plugins, cost to test here is high
    assert(low == 0);
}

2. And if high price is There is a little smaller than FIX_MAX, the _anyDepeggedOutsidePool check in the refresh function will revert.

if (low == 0 || _anyDepeggedInPool() || _anyDepeggedOutsidePool()) {
    markStatus(CollateralStatus.IFFY);
}

And the CurveStableMetapoolCollateral overrides it:

 function _anyDepeggedOutsidePool() internal view virtual override returns (bool) {
    try this.tryPairedPrice() returns (uint192 low, uint192 high) {
        // {UoA/tok} = {UoA/tok} + {UoA/tok}
        uint192 mid = (low + high) / 2;

        // If the price is below the default-threshold price, default eventually
        // uint192(+/-) is the same as Fix.plus/minus
        if (mid < pairedTokenPegBottom || mid > pairedTokenPegTop) return true;
    }

So the uint192 mid = (low + high) / 2; will revert because of uint192 overflow. The CurveStableRTokenMetapoolCollateral.refresh() will revert without any catch.

Becuase RToken.redeemTo and redeemCustom need to call assetRegistry.refresh(); at the beginning, it will revert directly.

Tools Used

Manual review

Recommended Mitigation Steps

The Fix.plus can't handle the uint192 overflow error. Try to override _anyDepeggedOutsidePool for CurveStableRTokenMetapoolCollateral as:

unchecked {
    uint192 mid = (high - low) / 2 + low;
}

The assert assert(low <= high) in the RTokenAsset.tryPrice has already protected everything.

Assessed type

DoS

Lines of code

https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/plugins/assets/RTokenAsset.sol#L163-L175 https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/plugins/assets/RTokenAsset.sol#L53-L69 https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/p1/BasketHandler.sol#L329-L351

Vulnerability details

The RTokenAsset is an implementation of interface IRTokenOracle to work as a oracle price feed for the little RToken. RTokenAsset implements the latestPrice function to get the oracle price and saved time from the cachedOracleData, which is updated by _updateCachedPrice function:

function _updateCachedPrice() internal {
    (uint192 low, uint192 high) = price();

    require(low != 0 && high != FIX_MAX, "invalid price");

    cachedOracleData = CachedOracleData(
        (low + high) / 2,
        block.timestamp,
        basketHandler.nonce(),
        backingManager.tradesOpen(),
        backingManager.tradesNonce()
    );
}

The _updateCachedPrice gets the low and high prices from price(), and updates the oracle price to (low + high) / 2. And it checks low != 0 && high != FIX_MAX.

The RTokenAsset.price just uses the return of tryPrice as the low price and high price, if tryPrice reverts, it will return (0, FIX_MAX), which is a invalid pirce range for the oracle price check above. But if there is any underlying collateral's price oracle reverts, for example oracle timeout, the RTokenAsset.price will return a valid but untrue (low, high) price range, which can be described as low = true_price * A1 and high = FIX_MAX * A2, A1 is bh.quantity(oracle_revert_coll) / all quantity for a BU and A2 is the BasketRange.top / RToken totalSupply.

Impact

The RToken oracle price will be about FIX_MAX / 2 when any underlying collateral's price oracle is timeout. It is significantly more than the actual price. It will lead to a distortion in the price of collateral associated with the RToken, for example CurveStableRTokenMetapoolCollateral:

    pairedAssetRegistry = IRToken(address(pairedToken)).main().assetRegistry();
    
    function tryPairedPrice()
    ...
    {
        return pairedAssetRegistry.toAsset(pairedToken).price();
    }

Proof of Concept

RToken.tryPrice gets the BU (low, high) price from basketHandler.price() first. BasketHandler._price(false) core logic:

for (uint256 i = 0; i < len; ++i) {
    uint192 qty = quantity(basket.erc20s[i]);

    (uint192 lowP, uint192 highP) = assetRegistry.toAsset(basket.erc20s[i]).price();

    low256 += qty.safeMul(lowP, RoundingMode.FLOOR);

    if (high256 < FIX_MAX) {
        if (highP == FIX_MAX) {
            high256 = FIX_MAX;
        } else {
            high256 += qty.safeMul(highP, RoundingMode.CEIL);
        }
    }
}

And the IAsset.price() should not revert. If the price oracle of the asset reverts, it just returns (0,FIX_MAX). In this case, the branch will enter high256 += qty.safeMul(highP, RoundingMode.CEIL); first. And it won't revert for overflow because the Fixed.safeMul will return FIX_MAX directly if any param is FIX_MAX:

function safeMul(
    ...
) internal pure returns (uint192) {
    if (a == FIX_MAX || b == FIX_MAX) return FIX_MAX;

So the high price is FIX_MAX, and the low price is reduced according to the share of qty.

Return to the RToken.tryPrice, the following codes uses basketRange() to calculate the low and high price for BU:

BasketRange memory range = basketRange(); // {BU}

// {UoA/tok} = {BU} * {UoA/BU} / {tok}
low = range.bottom.mulDiv(lowBUPrice, supply, FLOOR);
high = range.top.mulDiv(highBUPrice, supply, CEIL);

And the only thing has to be proofed is range.top.mulDiv(highBUPrice, supply, CEIL) should not revert for overflow in unit192. Now highBUPrice = FIX_MAX, according to the Fixed.mulDiv, if range.top <= supply it won't overflow. And for passing the check in the RToken._updateCachedPrice(), the high price should be lower than FIX_MAX. So it needs to ensure range.top < supply.

The max value of range.top is basketsNeeded which is defined in RecollateralizationLibP1.basketRange(ctx, reg):

if (range.top > basketsNeeded) range.top = basketsNeeded;

And the basketsNeeded:RToken supply is 1:1 at the beginning. If the RToken has experienced a haircut or the RToken is undercollateralized at present, the basketsNeeded can be lower than RToken supply.

Tools Used

Manual review

Recommended Mitigation Steps

Add a BU price valid check in the RToken.tryPrice:

function tryPrice() external view virtual returns (uint192 low, uint192 high) {
    (uint192 lowBUPrice, uint192 highBUPrice) = basketHandler.price(); // {UoA/BU}
    require(lowBUPrice != 0 && highBUPrice != FIX_MAX, "invalid price");

Assessed type

Context

1. cbETH UoA should be USD

code lines: https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/plugins/assets/cbeth/CBETHCollateral.sol#L53-L56

According to the cbeth README.md, the UoA and ref of the chETH are both ETH, but the CBETHCollateral.tryPrice() uses a chainlinkFeed to get the UoA/ref. It is contradictory. The UoA should be USD.

2. SafeERC20.safeApprove can lead to Morpho deposit revert

code lines: https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/2023-07-reserve/protocol/contracts/plugins/assets/morpho-aave/MorphoTokenisedDeposit.sol#L59

The SafeERC20.safeApprove can only use to init an allowance, if the allowance!=0 it will revert.

require(
    (value == 0) || (token.allowance(address(this), spender) == 0),
    "SafeERC20: approve from non-zero to non-zero allowance"
);

3. MorphoTokenisedDeposit _claimAssetRewards

code lines: https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/2023-07-reserve/protocol/contracts/plugins/assets/morpho-aave/MorphoTokenisedDeposit.sol#L44

The MorphoTokenisedDeposit._claimAssetRewards() function is empty:

function _claimAssetRewards() internal virtual override {}

Although the Morpho uses a rewards scheme that requires the results of off-chain computation to be piped into an on-chain function, a empty _claimAssetRewards function will cause users to lose part of rewards when they withdraw if rewards are not synchronized for a long time.

Suggest to add a rewards last update timestamp check in the _claimAssetRewards function.

4. RewardableERC20Wrapper ERC777 reentrancy attack

code lines: https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/2023-07-reserve/protocol/contracts/plugins/assets/erc20/RewardableERC20Wrapper.sol#L44-L46

In the RewardableERC20Wrapper.deposit function, the _mint operation is called before underlying tokens transfer:

_mint(_to, _amount); // does balance checkpointing
underlying.safeTransferFrom(msg.sender, address(this), _amount);
_afterDeposit(_amount, _to);

And there is not a reentrancy guard. So if the underlying token is an ERC777 standard token, an attacker can register a tokensToSend hook on the ERC-1820 Registry. And the attacker can mint and use any number of wrapper tokens before transfering any underlying token to the RewardableERC20Wrapper.

5.The low price of RTokenAsset from price()/lotPrice() is lower than the actual value when the basket is not ready

code lines: https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/plugins/assets/RTokenAsset.sol#L178-L214

RTokenAsset.price()/lotPrice() uses basketRange() to get the BU range. And the basketRange() gets the basketsHeld from BasketHandler and uses RecollateralizationLibP1.basketRange to get the range with all the assets when the RToken is undercollateralized.

But if the basket is not ready ( BasketHandler.disabled = true which occurs during the basket switching, or there is any collateral is CollateralStatus.DISABLED ), the BasketHandler.basketsHeldBy will return BasketRange(FIX_ZERO, FIX_MAX). A range.bottom = 0 leads to every collateral uses the total balance to lift the range.bottom. And because the range.bottom is calculated in the pessimistic case:

uint192 anchor = ctx.quantities[i].mul(ctx.basketsHeld.bottom, FLOOR);

uint192 val = low.mul(bal - anchor, FLOOR);

// (3) Buy BUs at their high price with the remaining value
// (4) Assume maximum slippage in trade
// {BU} = {UoA} * {1} / {UoA/BU}
range.bottom += val.mulDiv(FIX_ONE.minus(ctx.maxTradeSlippage), buPriceHigh, FLOOR);

The delta value is divided by the high BU price as the increments of range.bottom. So using the total collateral balance, instead of the bal - anchor vaule(bacuase the anchor is 0 now), will lead to the range.bottom is lower than the actual value.

6. The interface IRewardable is not specified in the contract definition of ConvexStakingWrapper

code lines: https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/plugins/assets/curve/cvx/vendor/ConvexStakingWrapper.sol#L44

CurveStableCollateral.claimRewards needs the config.erc20 should be an IRewardable implementation:

function claimRewards() external override(Asset, IRewardable) {
    IRewardable(address(erc20)).claimRewards();
}

But ConvexStakingWrapper doesn't specify this in the definition. Although it has implemented the claimRewards function.

7. Curve Read-only Reentrancy can increase the price of some CurveStableCollateral

If the curve pool of a CurveStableCollateral is a Plain Pool with a native gas token, just like eth/stETH pool: https://etherscan.io/address/0xdc24316b9ae028f1497c275eb9192a3ea0f67022#code the price can be manipulated by Curve Read-only Reentrancy.

A example is eth/stETH pool, in its remove_liquidity function:

# snippet from remove_liquidity
CurveToken(lp_token).burnFrom(msg.sender, _amount)
for i in range(N_COINS):
    value: uint256 = amounts[i] * _amount / total_supply
    if i == 0:
        raw_call(msg.sender, b"", value=value)
    else:
    assert ERC20(self.coins[1]).transfer(msg.sender, value)

First, LP tokens are burned. Next, each token is transferred out to the msg.sender. Given that ETH will be the first coin transferred out, token balances and total LP token supply will be inconsistent during the execution of the fallback function.

The CurveStableCollateral uses total underlying token balance value / lp supply to calculate the lp token price:

    (uint192 aumLow, uint192 aumHigh) = totalBalancesValue();

    // {tok}
    uint192 supply = shiftl_toFix(lpToken.totalSupply(), -int8(lpToken.decimals()));
    // We can always assume that the total supply is non-zero

    // {UoA/tok} = {UoA} / {tok}
    low = aumLow.div(supply, FLOOR);
    high = aumHigh.div(supply, CEIL);

So the price will be higher than the actual value becuase the other assets(except eth) are still in the pool but the lp supply has been cut down during the remove_liquidity fallback.