CBEthCollateral and AnkrStakedEthCollateral _underlyingRefPerTok is incorrect

[code423n4]

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

[c4-judge]

thereksfour marked the issue as primary issue

[tbrent]

This feels like a duplicate of #32. The root cause is an incorrect reference unit. The reference unit should be staked eth2, as indicated here.

[c4-sponsor]

pmckelvy1 marked the issue as sponsor confirmed

[c4-judge]

thereksfour marked the issue as satisfactory

[c4-judge]

thereksfour marked the issue as selected for report

[5z1punch]

This issue and https://github.com/code-423n4/2023-07-reserve-findings/issues/32 explain the misuse of tar unit and ref unit in staked eth related assets from different perspectives. The root cause is same, that 1 staked eth2 != 1 eth. This issue assumes that the ref token and target is all eth, which is referred to the docs https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/2023-07-reserve/protocol/contracts/plugins/assets/cbeth/README.md. So the error should be in the function _underlyingRefPerTok. But the issue https://github.com/code-423n4/2023-07-reserve-findings/issues/32 assumes that the ref unit should be staked eth2 and the target uint is eth. So it needs to modify function targetPerRef. I also have mentioned this mitigation in the Recommended Mitigation Steps section of the current issue:

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

In addition, I think the current issue is more appropriate to the document than the other one.

[c4-judge]

thereksfour changed the severity to 3 (High Risk)