`Reth.sol` can be partially drained via price manipulation

[code423n4]

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/derivatives/Reth.sol#L211-L216

Vulnerability details

Impact

Inconsistency between pricing used to determine amount of SafEth to mint and actual amount of rETH tokens obtained allows attacker to manipulate oracle and partially drain Reth.sol rETH balance.

Proof of Concept

Reth.ethPerDerivative returns ETH/rETH pricing of the deposit pool if the _amount passed can be deposited, otherwise it returns the current Uniswap price.

// Reth.sol:L211-216
function ethPerDerivative(uint256 _amount) public view returns (uint256) {
    if (poolCanDeposit(_amount))
        return
            RocketTokenRETHInterface(rethAddress()).getEthValue(10 ** 18);
    else return (poolPrice() * 10 ** 18) / (10 ** 18);
}

SafEth.stake determines the amount of tokens to mint based on the underlying value of the derivatives contracts.

// stake() - SafEth.sol:L68-99
uint256 underlyingValue = 0;

// Getting underlying value in terms of ETH for each derivative
for (uint i = 0; i < derivativeCount; i++)
    underlyingValue +=
        (derivatives[i].ethPerDerivative(derivatives[i].balance()) *
            derivatives[i].balance()) /
        10 ** 18;

uint256 totalSupply = totalSupply();
uint256 preDepositPrice; // Price of safETH in regards to ETH
if (totalSupply == 0)
    preDepositPrice = 10 ** 18; // initializes with a price of 1
else preDepositPrice = (10 ** 18 * underlyingValue) / totalSupply;

uint256 totalStakeValueEth = 0; // total amount of derivatives worth of ETH in system
for (uint i = 0; i < derivativeCount; i++) {
    uint256 weight = weights[i];
    IDerivative derivative = derivatives[i];
    if (weight == 0) continue;
    uint256 ethAmount = (msg.value * weight) / totalWeight;

    // This is slightly less than ethAmount because slippage
    uint256 depositAmount = derivative.deposit{value: ethAmount}();
    uint derivativeReceivedEthValue = (derivative.ethPerDerivative(
        depositAmount
    ) * depositAmount) / 10 ** 18;
    totalStakeValueEth += derivativeReceivedEthValue;
}
// mintAmount represents a percentage of the total assets in the system
uint256 mintAmount = (totalStakeValueEth * 10 ** 18) / preDepositPrice;
_mint(msg.sender, mintAmount);

The problem with this logic is that to retrieve the underlying value, the pricing of each derivative is retrieved with derivatives[i].ethPerDerivative(derivatives[i].balance(). Since the _amount passed to ethPerDerivative is the balance of the derivative contract, and not necessarily the amount being deposited, it's possible that the actual deposit amount is little enough that it can be deposited while the full balance of the contract may not be able to be deposited, causing a price discrepancy between the price used to determine the SafEth mint amount and the actual price used in obtaining rETH tokens. This price discrepancy opens up the following attack vector:

1. Attacker takes large rETH flashloan (or happens to be a RETH whale)
2. Attacker sells large amount of rETH in uniswap pool used in Reth.sol, causing the price to decrease significantly
3. Attacker calls stake with an amount which the rocket deposit pool can process when the total Reth.sol balance is greater than the rocket deposit pool can process
   1. Attacker receives amount of tokens to mint denominated partially in artificially reduced price of rETH in Uniswap pool, resulting in more SafEth being minted than corresponding value added throughout derivatives
4. Attacker reverses initial Uniswap trade, repaying flashloan if necessary
5. Attacker calls unstake to withdraw their share of SafEth now that the Uniswap pool is no longer being manipulated
   1. Attacker receives significantly more ETH than they initially staked
6. Attacker repeats the process as long as it's profitable, draining a significant portion of rETH from Reth.sol
   1. Can be repeated atomically such that pausing staking/unstaking will have no effect

Recommended Mitigation Steps

Ensure pricing is always consistent between execution and retrieving underlying value.

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #1004

[c4-judge]

Picodes marked the issue as not a duplicate

[c4-judge]

Picodes marked the issue as duplicate of #1125

[c4-judge]

Picodes marked the issue as satisfactory