Ethena Labs - peanuts's results

Lines of code

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

Vulnerability details

Impact

stUSDe can still be withdrawn for SOFT_RESTRICTED users, which breaks protocol intention.

Proof of Concept

Document states:

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.

Note that SOFT_RESTRICTED users cannot deposit USDe for stUSDe or redeem stUSDe for USDe. From the code in StakedUSDe.sol, it is true that SOFT_RESTRICTED users cannot deposit USDe for stUSDE. _deposit() is overriden from ERC4626. This means that when the user calls deposit() or mint(), which is a public function, _deposit() will be called. _deposit() will revert if the user has a SOFT_RESTRICTED role.

  function _deposit(address caller, address receiver, uint256 assets, uint256 shares)
    internal
    override
    nonReentrant
    notZero(assets)
    notZero(shares)
  {
    //@audit checks the SOFT_RESTRICTED role
    if (hasRole(SOFT_RESTRICTED_STAKER_ROLE, caller) || hasRole(SOFT_RESTRICTED_STAKER_ROLE, receiver)) {
      revert OperationNotAllowed();
    }
    super._deposit(caller, receiver, assets, shares);
    _checkMinShares();
  }

However, the same cannot be said for withdrawals. _withdraw() is also overridden, but this time, FULL_RESTRICTED_STAKER_ROLE is checked instead of SOFT_RESTRICTED_STAKER_ROLE. This means that SOFT_RESTRICTED users can still withdraw/redeem stUSDe for USDe.

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

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

There is no need to restrict FULL_RESTRICTED in deposit and withdraw, since the _beforeTransferToken hook takes care of the restrictions already. _deposit() in ERC4626 will call _mint() in ERC20, which will run the hook. If msg.sender or receiver is FULL_RESTRICTED, revert the function.

Tools Used

VSCode

Recommended Mitigation Steps

Recommend changing FULL_RESTRICTED_STAKER_ROLE to SOFT_RESTRICTED_STAKER_ROLE in the overridden _withdraw() function, like how _deposit() does in StakedUSDe.sol.

  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();
-    }

+    if (hasRole(SOFT_RESTRICTED_STAKER_ROLE, caller) || hasRole(SOFT_RESTRICTED_STAKER_ROLE, receiver)) {
+      revert OperationNotAllowed();
+    }

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

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2023-10-ethena/blob/ee67d9b542642c9757a6b826c82d0cae60256509/contracts/StakedUSDeV2.sol#L78-L90

Vulnerability details

Proof of Concept

The admin can choose whether to have a cooldown period when withdrawing/redeeming stUSDe. If there is a cooldown period and the admin sets it to zero (say for a special event or perhaps a breach in contract), users who already called cooldownAssets() or cooldownShares() will not be able to withdraw their USDe inside the silo.

  function cooldownAssets(uint256 assets, address owner) external ensureCooldownOn returns (uint256) {
    if (assets > maxWithdraw(owner)) revert ExcessiveWithdrawAmount();
    uint256 shares = previewWithdraw(assets);
    cooldowns[owner].cooldownEnd = uint104(block.timestamp) + cooldownDuration;
    cooldowns[owner].underlyingAmount += assets;
    _withdraw(_msgSender(), address(silo), owner, assets, shares);
    return shares;
  }

If cooldown is set to zero, users can call the withdraw() function immediately to convert stUSDe to USDe.

  function withdraw(uint256 assets, address receiver, address owner)
    public
    virtual
    override
    ensureCooldownOff
    returns (uint256)
  {
    return super.withdraw(assets, receiver, owner);
  }

But those who already initiated withdrawals will have to wait until the cooldown is finished to withdraw from the silo via unstake().

  function unstake(address receiver) external {
    UserCooldown storage userCooldown = cooldowns[msg.sender];
    uint256 assets = userCooldown.underlyingAmount;

    if (block.timestamp >= userCooldown.cooldownEnd) {
      userCooldown.cooldownEnd = 0;
      userCooldown.underlyingAmount = 0;

      silo.withdraw(receiver, assets);
    } else {
      revert InvalidCooldown();
    }
  }

Impact

USDe will still be locked in the silo when cooldown duration is set to 0.

Tools Used

VSCode

Recommended Mitigation Steps

Recommend letting those that have their USDe locked inside the silo contract to be able to immediately withdraw when cooldown is set to zero.

  function unstake(address receiver) external {
    UserCooldown storage userCooldown = cooldowns[msg.sender];
    uint256 assets = userCooldown.underlyingAmount;
    
    //@audit Add an optional route if cooldownDuration is 0
    if (cooldownDuration == 0) {
      userCooldown.cooldownEnd = 0;
      userCooldown.underlyingAmount = 0;
      silo.withdraw(receiver,assets);
      break;
    }

    if (block.timestamp >= userCooldown.cooldownEnd) {
      userCooldown.cooldownEnd = 0;
      userCooldown.underlyingAmount = 0;


      silo.withdraw(receiver, assets);
    } else {
      revert InvalidCooldown();
    }
  }

Assessed type

Timing

Lines of code

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

Vulnerability details

Proof of Concept

When any collateral, eg stUSDe is converted to USDe, yield is already being generated either through LSD rebase or perps contracts.

From documentation:

Users mint USDe with stETH, and Ethena opens an equvilant short ETH perps position on perps exchanges. stETH yields 3-4% annualized, while short ETH perps yield 6-8%.

The combined long and short position daily yield is sent to an insurance fund, and then sent to the staking contract every 8 hours.

The rewarder can deposit USDe into the staking contract through StakedUSDe.transferInRewards().

  function transferInRewards(uint256 amount) external nonReentrant onlyRole(REWARDER_ROLE) notZero(amount) {
    if (getUnvestedAmount() > 0) revert StillVesting();
    uint256 newVestingAmount = amount + getUnvestedAmount();

    vestingAmount = newVestingAmount;
    lastDistributionTimestamp = block.timestamp;
    // transfer assets from rewarder to this contract
    IERC20(asset()).safeTransferFrom(msg.sender, address(this), amount);

    emit RewardsReceived(amount, newVestingAmount);
  }

stUSDe token uses the ERC4626 functionality. If USDe yield is sent to the staking contract before any staking happens, the accounting of shares will break.

Normal Accounting:

1. User A deposits 1e18 worth of USDe in the staking contract. deposit() in ERC4626 will be called which calls previewDeposit() and subsequently _convertToShares(). This is the _convertToShares() formula.

    function _convertToShares(uint256 assets, Math.Rounding rounding) internal view virtual returns (uint256) {
        return assets.mulDiv(totalSupply() + 10 ** _decimalsOffset(), totalAssets() + 1, rounding);
    }

User A will get 1e18 * (0+1) / (0+1) = 1e18 * 1 / 1 = 1e18 worth of stUSDe shares.

2. Next, User B deposits 1e18 worth of USDe in the staking contract.

User B will get 1e18 * (1e18 + 1) / (1e18 + 1) = 1e18 worth of stUSDe shares

3. The REWARDER deposits 1e18 worth of USDe as rewards (increases the totalAssets, but not totalSupply)

Current totalAssets: 3e18 (USDe) Current totalSupply: 2e18 (stUSDe)

4. User A withdraws 1e18 worth of stUSDe shares. _convertToAssets() is called

    function _convertToAssets(uint256 shares, Math.Rounding rounding) internal view virtual returns (uint256) {
        return shares.mulDiv(totalAssets() + 1, totalSupply() + 10 ** _decimalsOffset(), rounding);
    }

User A will get 1e18 * (3e18 + 1) / (2e18 + 1) = 1e18 * 3e18 / 2e18 = 1.5e18 worth of USDe

5. User B withdraws 1e18 worth of stUSDe shares.

User B will get 1e18 * (1.5e18 + 1) / (1e18 + 1) = 1.5e18 worth of USDe.

This is the normal accounting, both of them gets 1.5e18 USDe when depositing 1e18 USDe and rewarder depositing 1e18 USDe.

Now, let's look at what happens when the rewarder deposits the yield first

1. REWARDER deposits 1e18 worth of USDe.

Total assets = 1e18 (USDe) Total supply = 0 (stUSDe)

2. User A deposits 1e18 worth of USDe.

assets.mulDiv(totalSupply() + 10 ** _decimalsOffset(), totalAssets() + 1, rounding);

1e18 * 1 / 1e18 = 1 stUSDe share, which will be reverted since min shares received must be 1e18. Since totalSupply() is still 0, the shares returned will always be <1e18, which will revert.

_checkMinShares() after depositing USDe.

  function _checkMinShares() internal view {
    uint256 _totalSupply = totalSupply();
    if (_totalSupply > 0 && _totalSupply < MIN_SHARES) revert MinSharesViolation();
  }

(This is also a problem if someone directly deposits USDe into the contract before any stUSDe shares are minted)

Impact

If USDe rewards is transferred before any stUSDe is minted, the shares distribution will break.

Tools Used

VSCode

Recommended Mitigation Steps

Make sure USDe is not transferred before any stUSDe tokens are minted. Best if there is a check to ensure that some stUSDe tokens are already minted / the protocol mints them, before depositing USDe directly into the staking contract.

Assessed type

ERC4626

[L-01] Line 91 in StakedUSDe.sol is redundant as getUnvestedAmount will always be zero.

In StakedUSDe.transferInRewards(), the first line checks if the return value of getUnvestedAmount() is greater than 0. If the return value is greater, revert. Else, continue. The second addition line is redundant as getUnvestedAmount() will always be 0.

  function transferInRewards(uint256 amount) external nonReentrant onlyRole(REWARDER_ROLE) notZero(amount) {
    //@audit getUnvestedAmount must return 0
    if (getUnvestedAmount() > 0) revert StillVesting();
    //@audit If getUnvestedAmount can only return 0 for function to proceed, this next addition is redundant since getUnvestedAmount always returns 0
    uint256 newVestingAmount = amount + getUnvestedAmount();

Remove the second addition line.

    uint256 newVestingAmount = amount + getUnvestedAmount();

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

[L-02] OpenZeppelin does not use hooks in the latest 5.0 version release

In 5.0, token hooks were removed from ERC20, ERC721, and ERC1155, favoring a single _update function. This can also be overridden for custom behaviors in any mint, transfer, or burn operation.

Take note when compiling the contracts or upgrading to a new contract. The override of _beforeTokenTransfer() will not work and the full restricted blacklisted users can still trade their tokens. Best practice is to override the _update function in the new ERC20 contract and use the latest openzeppelin version.

https://blog.openzeppelin.com/introducing-openzeppelin-contracts-5.0

[L-03] Benefactor and beneficiary makes is unnecessarily confusing and creates additional steps for token transfer

In EthenaMinting, there is a benefactor and beneficiary, which I assume one is the user and one is a trusted address. For the user to get his token, he must wait for the minter to mint his tokens, and then wait for the beneficiary to give him his USDe. There will be a lot of waiting time, which increases the centralization risks of the protocol. It may be hard for the user to call mint() directly since the protocol has to specify how much USDe they can mint for their given collateral, but the USDe should be directly minted to the user themselves.

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L431

[N-01] Best practice is to always call super._beforeTokenTransfer when overriding the beforeTokenTransfer hook

Although the parent hook is empty now, it is good to call the parent function in case OpenZeppelin implements new functionalities in the parent contract.

Always call the parent’s hook in your override using super. This will make sure all hooks in the inheritance tree are called: contracts like ERC20Pausable rely on this behavior.

https://docs.openzeppelin.com/contracts/4.x/extending-contracts#rules_of_hooks

Overview of the protocol

• There are three types of tokens: Collateral tokens, USDe token and stUSDe token.
• Collateral tokens consist of Liquid Staking Derivatives (only stETH for now, more LSD or other tokens in the future)
• USDe token is a ERC20 stablecoin created by Ethena. It does not gain yield. Think of it like USDC.
• stUSDe (Staked USDe) is another ERC20 token that extends ERC4626 functionality. Holding stUSDe will gain yield like rETH. It is a non-rebasing yield bearing token, ie the amount of stUSDe will not increase over time but the value of stUSDe will increase over time. If 1 stUSDe == 1 USDe at the start, then 1 stUSDe may become 1.1 USDe in the future.
• USDe can be staked to get stUSDe. stUSDe can be burned for USDe.

How the protocol works

• User deposits collateral tokens (eg stETH) for USDe tokens. The conversion rate is done by Ethena themselves. User has to sign before proceeding with the transactions.
• When user deposits collateral tokens, these collateral tokens are transferred to custodians. The custodians will then hedge against the collateral tokens to maintain a delta neutral position. All this is done off chain (or rather, off protocol). The extra yield from rebasing / hedging will be converted to stUSDe yield.
• The user has two choices with USDe. Firstly, they can hold onto USDe and use it like a normal stable coin. Secondly, they can stake their USDe tokens and get stUSDe tokens (staked USDe tokens). USDe tokens does not earn yield. stUSDe tokens earns yield.
• If a user chooses to stake USDe, he should be aware of the cooldown process when redeeming back his USDe. When staking USDe, the user will get back stUSDe. If the user wants to get back his USDe, he will have to burn stUSDe. If there is a cooldown process (the admin decides the duration of the cooldown, from 0 seconds -> 90 days), the user has to wait up to 90 days to get back his USDe token.

Example process

• User has 10 stETH.
• User deposits 10stETH and gets 20,000 USDe (conversion is determined by Ethena)
• User deposits 20,000 USDe and gets 20,000 shares of stUSDe. (stUSDe uses ERC4626)
• One year later, user wants to get back his USDe. He burns 20,000 stUSDe for 21,000 USDe.
• Since cooldown period is active, user has to wait 90 days to get his 21,000 USDe.
• User wants to get back his stETH. he signs a transaction and burns 21,000 USDe to get back 10.5 stETH. (conversion is determined by Ethena)

End to end process (code)

Depositing collateral (stETH) and getting USDe

• mint() in EthenaMinting.sol is called. Take note that the minter is calling the function, and not the user. The user probably has to approve some functions for the contract to use his assets
• collateral is transferred to the custodian address, according to the ratios provided (ratios must equate to 10,000)
• USDe is minted to the beneficiary
• I assume that there is another call for the user to claim their USDe from the beneficiary or that the beneficiary will transfer the USDe to the user off-code

Redeeming collateral (stETH) from USDe

• redeem() in EthenaMinting.sol is called. Once again, the minter is calling the function, and not the user.
• USDe is burned from the benefactor, which I assume is the user.
• Collateral is transferred to the beneficiary, which is Ethena. There should be another function for the user to get his collateral back.

Minting stUSDe

• Call deposit() directly from ERC4626.
• USDe will be deposited into StakedUSDe.sol and the appropriate stUSDe shares will be minted for the user

Withdrawing stUSDe

• cooldownAssets() is called in StakedUSDeV2. User inputs how much assets (USDe) he wants to get back and burns the appropriate amount of shares (stUSDe). For example, if user wants to get back 1000 USDe, he has to burn 100 stUSDe shares. ensureCooldownOn modifier is called to make sure that there is a cooldown set. Cooldowns mapping is updated, and _withdraw() in StakedUSDe.sol will be called.
• _withdraw() checks whether the assets and shares are non zero value, checks if the caller or receive has FULL_RESTRICTED_STAKER_ROLE before calling ERC4626 _withdraw().
• ERC4626 _withdraw() will burn the shares from the owner and transfer the asset to the receiver. If cooldown is on, USDe will be transferred to the silo contract. If cooldown is off, USDe will be directly transferred to the receiver.
• _withdraw() in StakedUSDe.sol will check whether the total supply of stUSDe is above 1e18 to prevent inflation attack.
• If there is a cooldown, once cooldown is over, unstake() in StakedUSDeV2 is called. Function will check whether cooldown is over and transfer userCooldown.underlyingAmount of USDe to the user. Cooldown mapping is set to zero.

Codebase quality analysis and review

Centralization Risks

Mechanism Review

StakedUSDeV2.sol

• withdraw() and redeem() is overriden from ERC4626, which is correct. All sUSDe withdrawals / redemption for USDe must wait for the cooldown duration. The USDe will be held in the silo contract.
• Admin can set the cooldown duration. If cooldown is off, user can immediately withdraw stUSDe for USDe. If cooldown is on, user will burn stUSDe for USDe which will be sent to the silo contract for safekeeping until the cooldown is over. User must wait for the cooldown to get their USDe back.

StakedUSDe.sol

• _beforeTokenTransfer() is overriden from ERC20. Take note that this function is deprecated in the newest OZ 5.0 version. (_update is used instead). If the protocol has a new StakedUSDe version, make sure the that OZ version used is <5.0 or update the hook.
• Role holders cannot renounce role. Prevents restricted role from renouncing their own role. Does not matter if other important roles get compromised, because admin can directly call grantRole() again.
• redistributeLockedAmount(). Burns the stUSDe amount from a fully restricted user and mints the same amount to an address of the admin's choice. Extremely centralized, but probably needed for legal reasons?

Preliminary Questions

1. Does USDe get burned for stUSDe or is stUSDe an extra token?
2. What is asset() in StakedUSDe.sol? USDe or stUSDe?
3. Quite confused with the getUnvestedAmount() function
4. What can FULL_RESTRICTED user do?
5. What can SOFT_RESTRICTED user do?
6. Who is the Benefactor?
7. Who is the Beneficiary?
8. Where is the approval to transfer collateral / burn USDe since Ethena is calling burn / redeem?

Preliminary Answers

1. USDe does not get burned for stUSDe. Users deposit USDe into the StakedUSDe.sol contract and get back stUSDe. The deposit function to deposit USDe is found in ERC4626 abstract contract. Yes, stUSDe is another token.
2. USDe is the asset(). stUSDe is the share. stUSDe is another ERC20 token.
3. This is to prevent users from staking USDe and immediately withdrawing to game the reward system. Say there is no cooldown to withdraw. Malicious User has 10,000 USDe to stake. When the rewarder is going to deposit the rewards via transferInRewards(), the malicious user can frontrun this deposit by staking 10,000 USDe. Afterwards, he can immediately withdraw his staked USDe to get a portion of the rewards. To prevent this from happening, getUnvestedAmount() is used so that malicious user cannot take advantage of the rewards.
4. Only applicable for stUSDe tokens. FULL_RESTRICTED users cannot transfer their stUSDe tokens. This means they cannot partake in the staking/unstaking of USDe because they cannot receive or get USDe tokens (due to the override _beforeTokenTransfer hook). The only thing they can do is to burn their stUSDe tokens.
5. Only applicable for stUSDe tokens. SOFT_RESTRICTED user is not supposed to be able to deposit their USDe tokens for stUSDe or withdraw their stUSDe tokens for USDe. They can still get stUSDe tokens from the open market.
6. Judging by collateral transfer, benefactor transfers collateral to different custody addresses and the signer is the benefactor, so the benefactor should be the user
7. Transferring assets to the beneficiary when redeeming, after USDe is burned from the benefactor, so the beneficiary should be Ethena.
8. Approval is probably called publicly, from the front end and through the public approve function in ERC20. Approves the EthenaMinting contract to spend collateral / burn USDe.

Time spent:

30 hours