Asymmetry contest - handsomegiraffe's results

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/WstEth.sol#L87

Vulnerability details

Impact

The price of safETH is determined by the prices of ETH derivatives (e.g. rETH) in ETH terms. For one of the derivatives, WstETH, price is returned in stETH terms instead of ETH. This could lead to serious consequences in the event that stETH price de-pegs from ETH.

Proof of Concept

// WstEth.sol
function ethPerDerivative(uint256 _amount) public view returns (uint256) {
        return IWStETH(WST_ETH).getStETHByWstETH(10 ** 18);
    }

In WstETH.sol , the price of WstETH is obtained from the Lido’s WstETH contract function getStETHByWstETH which returns the amount of stETH for one WstETH.

This is unexpected behaviour because in SafEth.sol , stake() expects to calculate underlyingValue in terms of ETH — not stETH.

// SafEth.sol
function stake() external payable {
        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;

In the event that stETH de-pegs from ETH (stETH worth less than ETH), the protocol would still assume a 1:1 ratio between stETH:ETH and calculate a higher-than-accurate underlyingValue, resulting in a user receiving less safETH than he deserves.

In the event that stETH de-pegs from ETH (stETH worth more than ETH), the protocol would still assume a 1:1 ratio between stETH:ETH and calculate a lower-than-accurate underlyingValue, resulting in a user receiving more safETH than he deserves. An attacker could take advantage of such a situation, to steal protocol’s funds by repeatedly minting safETH for cheap, and unstaking to receive ETH.

Example: 1 ETH = 0.5 steth 1 ETH = 1 rETH

1 ETH = 1 frxETH

Assume protocol has 1 rETH, 1 WstETH, 1 frxETH, and already minted 3 safETH -1) Attacker stakes(deposits) 1 ETH -2)underlyingValue = 2.5 ETH, totalSupply = 3, preDepositprice = 2.5 / 3 = 0.833 eth -3) totalStakeValueEth : 0.66 + 0.33 + 0.33 = 1.32 eth (assuming 33% weight each) -4) mintAmount = 1.32 / 0.833 = 1.5846 safETH

By depositing 1 ETH, attacker manage to mint 1.5846 safETH

Protocol now has 1.33 rETH, 1.66 WstETH, 1.33 frxETH, and 4.5846 safETH Attacker has roughly 1/3 of supply 5) Attacker unstakes 1.5846 safETH 6) Protocol withdraws 1/3 of each derivative and receives ~0.44 rETH, 0.44 frxETH and 0.55 WstETH 7) After converting to ETH, attacker receives ~1.9 ETH: stealing 0.9 ETH for a deposit of 1 ETH

Tools Used

Manual review

Recommended Mitigation Steps

In WstEth.sol, ethPerDerivative() after obtaining stETH for each WstETH, it should convert stETH to ETH. This can be done by obtaining stETH:ETH price from Chainlink for example.

// WstEth.sol
function ethPerDerivative(uint256 _amount) public view returns (uint256) {
        uint256 stEthAmt = IWStETH(WST_ETH).getStETHByWstETH(10 ** 18);
				return // convert stETHAmt to ETH using Chainlink
    }

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/SfrxEth.sol#L116 https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L215

Vulnerability details

Impact

The price of safETH is determined by the prices of ETH derivatives (e.g. rETH) in ETH terms. The protocol obtains the price of some ETH derivatives through decentralized oracles. These oracles are susceptible to price manipulation attacks, which subsequently would affect the price of safETH , leading to loss of protocol funds.

// SfrxEth.sol
function ethPerDerivative(uint256 _amount) public view returns (uint256) {
        uint256 frxAmount = IsFrxEth(SFRX_ETH_ADDRESS).convertToAssets(
            10 ** 18
        );
        return ((10 ** 18 * frxAmount) /
            IFrxEthEthPool(FRX_ETH_CRV_POOL_ADDRESS).price_oracle());
    }

Proof of Concept

Attack scenario: -1. Attacker flash loans large sum of DAI -2. Swap DAI for frxETH on Uniswap -3. Sell frxETH on Curve (frxETH - ETH pool) to depress price of frxETH -4. Go to Asymmetry to stake 1 ETH -5. In SafEth.sol , stake() function is called and underlyingValue of all derivatives are calculated to determine preDepositPrice -6. SfrxEth.sol returns ethPerDerivative as a low amount due to price manipulation in step 3 — therefore preDepositPrice is calculated as an artificially low value -7. Attacker mints large amount of safETH -8. Attacker unstake all his safETH and receives large amount of ETH -9. Attacker repays flash loan and keeps balance

Possible total loss of protocol’s funds, depending on how low the frxETH price can be manipulated, attacker may be able to mint a very large amount of safETH and steal all ETH in the protocol. Alternatively, attacker may carry out attack described above multiple times till all funds are drained.

This is similar to many previous other oracle manipulation hacks, the most recent one at Bonq DAO https://hacken.io/insights/bonqdao-hack/

Tools Used

Manual review Hardhat

Recommended Mitigation Steps

Do not solely rely one one decentralized oracle/AMM for price.

frxETH price is assessed to be the most susceptible as price is read from Curve pool oracle. rETH price, which may be read from UniswapV3, is less susceptible due to TWAP model. wstETH price is not read from an AMM oracle, but also has other flaws described in another finding, such as returning price in stETH terms not ETH.

It is recommended that all derivative prices are checked against other oracle sources (e.g. UniswapV3 or Chainlink) before they are consumed. Another guard could also be added to check if prices have drastic movements from a lastUpdatedPrice similar to how Liquity protocol handles oracle prices. https://www.liquity.org/blog/price-oracles-in-liquity

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/SfrxEth.sol#L94 https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/WstEth.sol#L73

Vulnerability details

Impact

Stake/Deposit actions do not specify any minimum accepted values. This renders users and the protocol prone to arbitrage attacks, causing them to receive less than expected value.

Proof of Concept

Using deposit() in the SfrxEth.sol contract:

// SfrxEth.sol
function deposit() external payable onlyOwner returns (uint256) {
        IFrxETHMinter frxETHMinterContract = IFrxETHMinter(
            FRX_ETH_MINTER_ADDRESS
        );
        uint256 sfrxBalancePre = IERC20(SFRX_ETH_ADDRESS).balanceOf(
            address(this)
        );
        frxETHMinterContract.submitAndDeposit{value: msg.value}(address(this));
        uint256 sfrxBalancePost = IERC20(SFRX_ETH_ADDRESS).balanceOf(
            address(this)
        );
        return sfrxBalancePost - sfrxBalancePre;
    }

If the frxETHMinterContract.submitAndDeposit returns a very low amount (due to arbitrage or error on Frax’s contract), the protocol will assume very little was deposited and mint less safETH to the user who staked.

Tools Used

Manual review

Recommended Mitigation Steps

It is advised to add a param to stake() to relay expected minimum receive values and revert if the minimum is not received. For example:

// SafEth.sol
function stake(uint256 minReceiveAmt) external payable {
...
uint256 mintAmount = (totalStakeValueEth * 10 ** 18) / preDepositPrice;
require(mintAmount >= minReceiveAmt, "less than minimum received") 
_mint(msg.sender, mintAmount);