Ethena Labs - pep7siup's results

Lines of code

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

Vulnerability details

Impact

The impact of this vulnerability is significant as it allows users with the SOFT_RESTRICTED_STAKER_ROLE to withdraw or redeem stUSDe tokens for USDe, which contradicts the intended restrictions outlined in the project's documentation.

// File: README.md
98: 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. // <= FOUND

The project's README specifies that SOFT_RESTRICTED_STAKER_ROLE should not be able to withdraw stUSDe tokens for USDe. However, in the StakedUSDe._withdraw method, the FULL_RESTRICTED_STAKER_ROLE is incorrectly used as the withdrawal guard instead. This discrepancy between the code and the documentation creates a serious inconsistency in the project, making it possible for US users to purchase stUSDe from the open market, farm rewards over time, and then redeem stUSDe for USDe. This violates the legal requirements set out in the documentation and can lead to an unauthorized conversion of assets.

Proof of Concept

The root cause of the vulnerability is that the _withdraw method, which facilitates the withdrawal or redemption of stUSDe tokens, uses FULL_RESTRICTED_STAKER_ROLE as the guard against withdrawals. This method should use SOFT_RESTRICTED_STAKER_ROLE instead. Let's walk through the issue with the following scenario:

1. Alice is a user with the SOFT_RESTRICTED_STAKER_ROLE and is therefore not supposed to withdraw or redeem stUSDe tokens for USDe.

2. The project's README and documentation specify that users with the SOFT_RESTRICTED_STAKER_ROLE should not be allowed to perform these actions.

3. However, the StakedUSDe._withdraw method uses FULL_RESTRICTED_STAKER_ROLE as the guard for withdrawals.

4. This means that even though Alice has the SOFT_RESTRICTED_STAKER_ROLE, she is still able to withdraw or redeem stUSDe tokens for USDe.

5. Alice takes advantage of this vulnerability by purchasing stUSDe tokens from the open market and farming rewards over time.

6. When Alice attempts to redeem her stUSDe tokens for USDe, she successfully converts her assets, which is against the intended restrictions and legal requirements.

7. As a result, the vulnerability allows unauthorized conversion of assets, putting the project at risk of inconsistencies with the provided documentation and restrictions.

Tools Used

Manual Review

Recommended Mitigation Steps

To mitigate this vulnerability and maintain consistency with the documentation and intended restrictions, it is recommended to fix the following line in the _withdraw method:

- if (hasRole(FULL_RESTRICTED_STAKER_ROLE, caller) || hasRole(FULL_RESTRICTED_STAKER_ROLE, receiver))

+ if (hasRole(SOFT_RESTRICTED_STAKER_ROLE, caller) || hasRole(SOFT_RESTRICTED_STAKER_ROLE, receiver))

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2023-10-ethena/tree/main/contracts/StakedUSDeV2.sol#L100 https://github.com/code-423n4/2023-10-ethena/tree/main/contracts/StakedUSDeV2.sol#L116

Vulnerability details

Impact

This can potentially lead to the premature unstaking of a substantial amount of USDe tokens from the previous cooldowns, flooding the market and causing price instability. The vulnerability arises from the fact that the previous cooldownEnd is not properly verified against the new cooldownDuration, which should always extend the pending cooldown period rather than overriding it.

Proof of Concept

diff --git a/test/foundry/staking/StakedUSDeV2.cooldownEnabled.t.sol b/test/foundry/staking/StakedUSDeV2.cooldownEnabled.t.sol
index 959d1b2..3eec934 100644
--- a/test/foundry/staking/StakedUSDeV2.cooldownEnabled.t.sol
+++ b/test/foundry/staking/StakedUSDeV2.cooldownEnabled.t.sol
@@ -520,6 +520,39 @@ contract StakedUSDeV2CooldownTest is Test, IERC20Events {
     stakedUSDe.cooldownShares(0, address(0));
   }
 
+  function test_premature_unstake() public {
+    _mintApproveDeposit(alice, 10 ether);
+    vm.prank(owner);
+    stakedUSDe.setCooldownDuration(2 weeks);
+
+    vm.startPrank(alice);
+    // 1st cooldown: cooldownEnd = blocktime + 2 weeks
+    stakedUSDe.cooldownAssets(1 ether, alice);
+    (uint104 cooldownEnd, ) = stakedUSDe.cooldowns(alice);
+    assertEq(cooldownEnd, uint104(block.timestamp) + 2 weeks);
+    vm.stopPrank();
+
+    // Owner changes cooldown duration to 1 day
+    vm.prank(owner);
+    stakedUSDe.setCooldownDuration(1 days);
+
+    // 2nd cooldown: cooldownEnd = blocktime + 1 days
+    vm.startPrank(alice);
+    stakedUSDe.cooldownAssets(1 ether, alice);
+    (cooldownEnd, ) = stakedUSDe.cooldowns(alice);
+    assertEq(cooldownEnd, uint104(block.timestamp) + 1 days);
+
+    vm.warp(block.timestamp + 1 days);
+
+    uint beforeBalance = usdeToken.balanceOf(alice);
+    stakedUSDe.unstake(alice);
+    uint afterBalance = usdeToken.balanceOf(alice);
+
+    // After ONLY 1 day, alice can withdraw all 2 ether of assets, disregards the 2-week cooldown constraints from the 1st request
+    assertEq(afterBalance - beforeBalance, 2 ether);
+    vm.stopPrank();
+  }
+
   function testFuzzCooldownAssets(uint256 amount) public {
     amount = bound(amount, 1 ether, 1e40);
     _mintApproveDeposit(alice, amount);

Results:

forge test -vvv --mt test_premature_unstake [PASS] test_premature_unstake() (gas: 276015)

This confirms that the cooldown amount which was supposed to be unstaked after a 2-week period, could be entirely withdrawn after ONLY 1 day.

Tools Used

Foundry test

Recommended Mitigation Steps

The contract should verify that the new cooldownDuration extends the pending cooldowns[user].cooldownEnd period, rather than shortening or reseting it.

Assessed type

Invalid Validation