Ethena Labs - ayden's results

Lines of code

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

Vulnerability details

Impact

Malicious users can transfer USDe token to StakedUSDe protocol directly lead to a denial of service (DoS) for StakedUSDe due to the limit shares check .

Proof of Concept

User deposit USDe token to StakedUSDe protocol to get share via invoke external deposit function. Let's see how share is calculate :

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

Since decimalsOffset() == 0 and totalAssets equal the balance of USDe in this protocol

    function totalAssets() public view virtual override returns (uint256) {
        return _asset.balanceOf(address(this));
    }

The minimum share is set to 1 ether。

  uint256 private constant MIN_SHARES = 1 ether;

Assuming malicious users transfer 1 ether of USDe into the protocol and receive ZERO shares, how much tokens does the next user need to pay if they want to exceed the minimum share limit of 1 ether? That would be 1 ether times 1 ether, which is a substantial amount.

I add a test case in StakedUSDe.t.sol:

  function testMinSharesViolation() public {
    address malicious = vm.addr(100);

    usdeToken.mint(malicious, 1 ether);
    usdeToken.mint(alice, 1000 ether);

    //assume malicious user deposit 1 ether into protocol.
    vm.startPrank(malicious);
    usdeToken.transfer(address(stakedUSDe), 1 ether);

    
    vm.stopPrank();
    vm.startPrank(alice);
    usdeToken.approve(address(stakedUSDe), type(uint256).max);

    //1000 ether can't exceed the minimum share limit of 1 ether
    vm.expectRevert(IStakedUSDe.MinSharesViolation.selector);
    stakedUSDe.deposit(1000 ether, alice);
  }

We can see even alice deposit a substantial number of tokens but still cannot surpass the 1 ether share limit which will lead to a denial of service (DoS) for StakedUSDe due to MinShares checks

Tools Used

vscode

Recommended Mitigation Steps

We can solve this issue by setting a minimum deposit amount

Assessed type

DoS

1.should use EIP-1271 Standard Signature Validation Method for Contracts Standard way to verify a signature when the account is a smart contract https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L342 Recommend to use EIP-1271 Standard to verify a signature when the account is a smart contract

2.unnecessary bitwise or opration https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L394 If invalidator not it zero transaction must be revert in verifyNonce , thus invalidator == 0 , The bitwise OR operation with 0 on any number always results in the same number, so the OR operation is unnecessary

3.unnecessary addition operation If the result of getUnvestedAmount is bigger than zero the transaction must be revert. So blow addition operation is unnecessary result in waste of gas https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/StakedUSDe.sol#L91

4.unstake not checked the unstake amount lead to waste of gas fee. https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/StakedUSDeV2.sol#L78#L90 user can invoke unstake directly without invoking cooldown.Since userCooldown.cooldownEnd ==0 , silo will withdraw zero amount assets which can lead to waste of gas fee.

-   if (block.timestamp >= userCooldown.cooldownEnd) { //@audit should ensure >0.
+   if (block.timestamp >= userCooldown.cooldownEnd && userCooldown.underlyingAmount>0) { 
      userCooldown.cooldownEnd = 0;
      userCooldown.underlyingAmount = 0;

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