Ethena Labs - Krace's results

Lines of code

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

Vulnerability details

Impact

A user with SOFT_RESTRICTED_STAKER_ROLE role could call withdraw to withdraw stUSDe for USDe without any restrictions. This is explicitly stated in the documentation as not being allowed.

Proof of Concept

According to the README, SOFT_RESTRICTED_STAKER_ROLE cannot deposit or withdraw.

Due to legal requirements, there's a SOFT_RESTRICTED_STAKER_ROLE and FULL_RESTRICTED_STAKER_ROLE. The former is for addresses based in countries we are not allowed to provide yield to, for example USA. Addresses under this category will be soft restricted. They cannot deposit USDe to get stUSDe or withdraw stUSDe for USDe. However they can participate in earning yield by buying and selling stUSDe on the open market.

However, the _withdraw doesn't check that owner is not a SOFT_RESTRICTED_STAKER_ROLE. A SOFT_RESTRICTED_STAKER_ROLE user could still call this function to withdraw stUSDe.

  function _withdraw(address caller, address receiver, address _owner, uint256 assets, uint256 shares)
    internal
    override
    nonReentrant
    notZero(assets)
    notZero(shares)
  {
  //@audit: owner should not be SOFT_RESTRICTED_STAKER_ROLE
    if (hasRole(FULL_RESTRICTED_STAKER_ROLE, caller) || hasRole(FULL_RESTRICTED_STAKER_ROLE, receiver)) {
      revert OperationNotAllowed();
    }

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

Test

Considering that alice has deposited some USDe and got some stUSDe, but then she is added to a black list with the role SOFT_RESTRICTED_STAKER_ROLE. However, she could still withdraw her stUSDe.

Add the below test to test/foundry/staking/StakedUSDe.t.sol and run it with forge test --match-test testSoftWithdraw -vv

  function testSoftWithdraw() public {
    bytes32 BLACKLIST_MANAGER_ROLE = keccak256("BLACKLIST_MANAGER_ROLE");
    vm.prank(owner);
    stakedUSDe.grantRole(BLACKLIST_MANAGER_ROLE, alice);

    uint256 amount = 2 ether;
    usdeToken.mint(alice, amount);

    vm.startPrank(alice);
    usdeToken.approve(address(stakedUSDe), amount);
    stakedUSDe.deposit(amount, alice);
    // add alice to blacklist
    stakedUSDe.addToBlacklist(alice, false);
    // alice can still withdraw
    stakedUSDe.withdraw(amount, alice, alice);

  }

Tools Used

Foundry

Recommended Mitigation Steps

Check the role of owner in _withdraw.

@@ -229,7 +231,7 @@ contract StakedUSDe is SingleAdminAccessControl, ReentrancyGuard, ERC20Permit, E
     notZero(assets)
     notZero(shares)
   {
-    if (hasRole(FULL_RESTRICTED_STAKER_ROLE, caller) || hasRole(FULL_RESTRICTED_STAKER_ROLE, receiver) || hasRole(SOFT_RESTRICTED_STAKER_ROLE, _owner)) {
+    if (hasRole(FULL_RESTRICTED_STAKER_ROLE, caller) || hasRole(FULL_RESTRICTED_STAKER_ROLE, receiver)) {
       revert OperationNotAllowed();
     }

Assessed type

Access Control