search

code-423n4 / 2024-04-renzo-validation

2 stars 2 forks source link

The user received less money than expected #372

Closed c4-bot-10 closed 5 months ago

c4-bot-10 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/RestakeManager.sol#L270-L358

Vulnerability details

Impact

The user will receive less money than they should have.

Proof of Concept

The user's withdrawal process involves two steps:

  1. Calling withdraw to initiate a withdraw request and calculate the amount to be received using the formula share * totalAsset / totalShare.
  2. Calling claim to burn ezETH (share) and receive the desired collateral token.

share: user own ezETH amount

totalAsset: TVL

totalShare: ezETH totalSupply

In this process, the amount received includes the funds from rewards. The calculation of TVL does not take into account the amount waiting to be claimed stored in the claimReserve state variable. Therefore, if a user initiates a withdrawal request but does not call the claim function, the TVL will always include the amount waiting to be claimed.

TVL:

https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/RestakeManager.sol#L270-L358

Here's an example:

  1. User A calls withdraw to initiate a withdrawal request.
  2. After claimReserve seconds, User A still does not call claim to receive the amount.
  3. Another day passes, and User B initiates a withdrawal request. However, when calculating the amount to be received, TVL still includes the amount waiting to be claimed by User A and totalSupply. In this process, rewards generated by RENZO continue to include User A, but User A cannot claim them (because User A's claimable amount has already been calculated), resulting in User B receiving less reward than expected when claiming the total amount, as User A is included in this process.

poc coding :

mock/mockOracle:

pragma solidity 0.8.19;

import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";
contract TestOracle is AggregatorV3Interface{
    constructor() {

    }

    function decimals() external override view returns (uint8) {
        return 18;
    }

    function description() external override view returns (string memory) {
        return "none";
    }

    function version() external override view returns (uint256) {
        return 1;
    }

    function getRoundData(
        uint80 _roundId
    ) external override view returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound) {
        return (0, 0, 0, 0, 0);
    }

    function latestRoundData() external override view returns (
            uint80 roundId,
            int256 answer,
            uint256 startedAt,
            uint256 updatedAt,
            uint80 answeredInRound
        ){
            return (0, 1000, 0, block.timestamp, 0);
    }
}

mock/mockStrategyManager.sol:

pragma solidity 0.8.19;
import "../../contracts/EigenLayer/interfaces/IStrategyManager.sol";

contract MockStrategyManager {
    constructor() {

    }
}

attack.t.sol:

pragma solidity 0.8.19;

import "forge-std/Test.sol";
import "forge-std/console.sol";
import {MockERC20} from "forge-std/mocks/MockERC20.sol";

import "./mockTests/mockOracle.sol";
import "./mockTests/mockStrategyManager.sol";

import "../contracts/Oracle/RenzoOracle.sol";
import "../contracts/Oracle/IRenzoOracle.sol";
import "../contracts/Permissions/RoleManager.sol";
import "../contracts/Permissions/IRoleManager.sol";
import {RoleManagerStorageV1} from "../contracts/Permissions/RoleManagerStorage.sol";
import "../contracts/EigenLayer/interfaces/IStrategy.sol";
import "../contracts/EigenLayer/interfaces/IStrategyManager.sol";
import "../contracts/EigenLayer/interfaces/IDelegationManager.sol";
import "../contracts/Deposits/DepositQueue.sol";
import "../contracts/Deposits/IDepositQueue.sol";
import "../contracts/Withdraw/WithdrawQueue.sol";
import {WithdrawQueueStorageV1} from "../contracts/Withdraw/WithdrawQueueStorage.sol";
import "../contracts/Withdraw/IWithdrawQueue.sol";
import "../contracts/RestakeManager.sol";
import "../contracts/IRestakeManager.sol";
import "../contracts/token/EzEthToken.sol";
import "../contracts/token/IEzEthToken.sol";

contract testAttack is Test {
    address admin;
    address alice;
    address bob;
    address joy;

    IEzEthToken public ezETH;
    address public stETH;
    address public cbETH;
    RoleManager public roleManager;
    address public renzoOracle;
    address public mockStrategyManager;
    IRestakeManager public restakeManager;
    DepositQueue public depositQueue;
    WithdrawQueue public withdrawQueue;

    function setUp() public {
        admin = makeAddr("Admin");
        alice = makeAddr("Alice");
        bob = makeAddr("bob");
        joy = makeAddr("joy");

        roleManager = deployRoleManager();
        vm.startPrank(admin);
        roleManager.grantRole(keccak256("RESTAKE_MANAGER_ADMIN"), address(admin));
        roleManager.grantRole(keccak256("RX_ETH_MINTER_BURNER"), address(admin));
        ezETH = IEzEthToken(deployEzETH(roleManager));
        (stETH,cbETH) = deployToken();
        renzoOracle = deployOracle(roleManager, IERC20(stETH), IERC20(cbETH));
        mockStrategyManager = address(new MockStrategyManager());
        depositQueue = deployDepositQueue(roleManager);
        restakeManager = IRestakeManager(deployRestakeManager(roleManager, ezETH, IRenzoOracle(renzoOracle), IStrategyManager(mockStrategyManager), IDelegationManager(address(0)), depositQueue));
        WithdrawQueueStorageV1.TokenWithdrawBuffer[] memory _withdrawalBufferTarget = new WithdrawQueueStorageV1.TokenWithdrawBuffer[](2);
        _withdrawalBufferTarget[0] = WithdrawQueueStorageV1.TokenWithdrawBuffer(stETH, 5 * 10**18);
        _withdrawalBufferTarget[1] = WithdrawQueueStorageV1.TokenWithdrawBuffer(address(0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE), 5 * 10**18);
        withdrawQueue = deployWithdraw(roleManager, restakeManager, ezETH, IRenzoOracle(renzoOracle), 1, _withdrawalBufferTarget);
        DepositQueue(depositQueue).setWithdrawQueue(IWithdrawQueue(address(withdrawQueue)));
        roleManager.grantRole(keccak256("RX_ETH_MINTER_BURNER"), address(withdrawQueue));
        roleManager.grantRole(keccak256("RX_ETH_MINTER_BURNER"), address(depositQueue));
        vm.stopPrank();
    }

    function deployRoleManager() public returns(RoleManager) {
        RoleManager role = new RoleManager();
        role.initialize(admin);
        return role;
    }

    function deployEzETH(IRoleManager _roleManager) public returns(address) {
        EzEthToken _ezETH = new EzEthToken();
        _ezETH.initialize(_roleManager);
        return address(_ezETH);
    }

    function deployToken() public returns(address,address){
        MockERC20 _stETH = new MockERC20();
        MockERC20 _cbETH = new MockERC20();

        _stETH.initialize("Staked ETH", "stETH", 18);
        _cbETH.initialize("Coinbase ETH", "cbETH", 18);

        return (address(_stETH), address(_cbETH));
    }

    function deployOracle(IRoleManager _roleManager, IERC20 _tokenA, IERC20 _tokenB) public returns(address) {
        RenzoOracle _renzoOracle = new RenzoOracle();

        roleManager.grantRole(keccak256("ORACLE_ADMIN"), admin);
        _renzoOracle.initialize(_roleManager);
        TestOracle tokenAOracle = new TestOracle();
        TestOracle tokenBOracle = new TestOracle();
        _renzoOracle.setOracleAddress(_tokenA, tokenAOracle);
        _renzoOracle.setOracleAddress(_tokenB, tokenBOracle);

        return address(_renzoOracle);
    }

    function deployRestakeManager(IRoleManager _roleManager, 
    IEzEthToken _ezETH, 
    IRenzoOracle _renzoOracle, 
    IStrategyManager _strategyManager, 
    IDelegationManager _delegationManager, 
    IDepositQueue _depositQueue) public returns(address){
        RestakeManager _restakeManager = new RestakeManager();
        _restakeManager.initialize(_roleManager, _ezETH, _renzoOracle, _strategyManager, _delegationManager, _depositQueue);
        return address(_restakeManager);
    }

    function deployOperatorDelegators() public {

    }

    function deployDepositQueue(IRoleManager _roleManager) public returns(DepositQueue){
        DepositQueue _depositQueue = new DepositQueue();
        _depositQueue.initialize(_roleManager);
        return _depositQueue;
    }

    function deployWithdraw(
        IRoleManager _roleManager,
        IRestakeManager _restakeManager,
        IEzEthToken _ezETH,
        IRenzoOracle _renzoOracle,
        uint256 _coolDownPeriod,
        WithdrawQueueStorageV1.TokenWithdrawBuffer[] memory _withdrawalBufferTarget
    ) public returns(WithdrawQueue){
        WithdrawQueue _withdrawQueue = new WithdrawQueue();
        _withdrawQueue.initialize(_roleManager, _restakeManager, _ezETH, _renzoOracle, _coolDownPeriod, _withdrawalBufferTarget);
        return _withdrawQueue;
    }

    function testReceiveLess1() external {
        vm.deal(address(withdrawQueue), 100 * 10**18);
        vm.startPrank(admin);

        ezETH.mint(alice, 10 * 10**18);
        ezETH.mint(bob, 10 * 10**18);
        ezETH.mint(joy, 10 * 10**18);

        console.log("start...");

        (, , uint256 totalTVL) = restakeManager.calculateTVLs();
        console.log("Before tvl: %d", totalTVL);
        console.log("Before alice ezETH: %d", ezETH.balanceOf(alice));
        console.log("Before bob   ezETH: %d", ezETH.balanceOf(bob));
        console.log("Before joy   ezETH: %d", ezETH.balanceOf(joy));

        vm.stopPrank();

        vm.startPrank(alice);
        ezETH.approve(address(withdrawQueue), 10 * 10 ** 18);
        withdrawQueue.withdraw(10 * 10 ** 18, address(0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE));
        vm.stopPrank();
        vm.warp(10);
        vm.deal(address(withdrawQueue), 200 * 10 ** 18);
        vm.startPrank(bob);
        ezETH.approve(address(withdrawQueue), 10 * 10 ** 18);
        withdrawQueue.withdraw(10 * 10 ** 18, address(0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE));
        vm.stopPrank();
        vm.warp(20);

        vm.startPrank(alice);
        ( , , uint256 amount, , ) = withdrawQueue.withdrawRequests(alice, 0);
        console.log("Step 1 --- alice: %d", amount);
        vm.stopPrank();
        vm.startPrank(bob);
        ( , , amount, , ) = withdrawQueue.withdrawRequests(bob, 0);
        console.log("Step 1 --- bob:   %d", amount);
        vm.stopPrank();
    }

    function testReceiveLess2() external {
        vm.deal(address(withdrawQueue), 100 * 10**18);
        vm.startPrank(admin);

        ezETH.mint(alice, 10 * 10**18);
        ezETH.mint(bob, 10 * 10**18);
        ezETH.mint(joy, 10 * 10**18);

        console.log("start...");

        (, , uint256 totalTVL) = restakeManager.calculateTVLs();
        console.log("Before tvl: %d", totalTVL);
        console.log("Before alice ezETH: %d", ezETH.balanceOf(alice));
        console.log("Before bob   ezETH: %d", ezETH.balanceOf(bob));
        console.log("Before joy   ezETH: %d", ezETH.balanceOf(joy));

        vm.stopPrank();

        vm.startPrank(alice);
        ezETH.approve(address(withdrawQueue), 10 * 10 ** 18);
        withdrawQueue.withdraw(10 * 10 ** 18, address(0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE));
        vm.stopPrank();

        vm.warp(10);

        vm.startPrank(admin);
        // Simulate Alice's status after withdrawing money
        vm.deal(address(withdrawQueue), 200 * 10**18 - 16666666666666666666);
        ezETH.burn(address(withdrawQueue), 10 * 10 ** 18);

        vm.stopPrank();

        vm.startPrank(bob);
        ezETH.approve(address(withdrawQueue), 10 * 10 ** 18);
        withdrawQueue.withdraw(10 * 10 ** 18, address(0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE));
        vm.stopPrank();

        vm.startPrank(alice);
        ( , , uint256 amount, , ) = withdrawQueue.withdrawRequests(alice, 0);
        console.log("Step 1 --- alice: %d", amount);
        vm.stopPrank();

        vm.startPrank(bob);
        ( , , amount, , ) = withdrawQueue.withdrawRequests(bob, 0);
        console.log("Step 1 --- bob:   %d", amount);
        vm.stopPrank();
    }
    /*
        struct WithdrawRequest {
        address collateralToken;
        uint256 withdrawRequestID;
        uint256 amountToRedeem;
        uint256 ezETHLocked;
        uint256 createdAt;
    }
     */
}

Results:

Logs:
  start...
  Before tvl: 100000000000000000000
  Before alice ezETH: 10000000000000000000
  Before bob   ezETH: 10000000000000000000
  Before joy   ezETH: 10000000000000000000
  Step 1 --- alice: 33333333333333333333
  Step 1 --- bob:   66666666666666666666

[PASS] testReceiveLess2() (gas: 542203)
Logs:
  start...
  Before tvl: 100000000000000000000
  Before alice ezETH: 10000000000000000000
  Before bob   ezETH: 10000000000000000000
  Before joy   ezETH: 10000000000000000000
  Step 1 --- alice: 33333333333333333333
  Step 1 --- bob:   91666666666666666667

run forge test -vvv

Tools Used

Manual Review

Recommended Mitigation Steps

When calculating TVL, subtract the total value of claimReserve.

Assessed type

Payable

0xJuancito commented 5 months ago

@howlbot accept