Ethena Labs - 0xWaitress's results

Lines of code

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

Vulnerability details

Impact

donation attack is still possible in the beginning of stakedUSDe deployment; such that the attacker can Ddos the withdrawal

_checkMinShares is only effective when shares > 0 and share is lower than 1 ether after a deposit. However the first depositor can still launch an inflation/donation attack, by:

1. donate a massive amount of stUSDe to the contract.

2. deposit to get 1 ether unit of shares depending on how much he donates in step 1.). Basically he/she still owns the whole pool since attacker is the first depositor.

3. Whoever deposit next, would get only a small shares (possibly 0.000x), in this case, if the depositor would like to withdraw, the attacker can frontrun the withdrawal such that the share is reduced to "just 1 ether". Then the victim would not be able to withdraw since the withdraw tx would fail to pass _checkMinShares due to redicing the totalSupply to lower than 1 ether

  /// @notice ensures a small non-zero amount of shares does not remain, exposing to donation attack
  function _checkMinShares() internal view {
    uint256 _totalSupply = totalSupply();
    if (_totalSupply > 0 && _totalSupply < MIN_SHARES) revert MinSharesViolation();
  }

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

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

Proof of Concept

Tools Used

Recommended Mitigation Steps

Consider remove the check in _withdraw.

Assessed type

DoS

Lines of code

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

Vulnerability details

Impact

stakedUSDe can be Ddos by donation attack in initial deployment to brick deposit

attacker can Ddos the stakedUSDe contract by donate a small amount to the contract when there is no depositor. Since there is a checkMinShare of 1 ether in any deposit, if the attacker donates, say 1 USDe to the contract, then anyone would need to deposit a minimum of 1e18 * 1e18 or 1e36 asset to get 1e18 shares, which is not possible since 1e36 exceeds any meaningful supply.

OpenZeppelin ERC4626 share conversion. https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/extensions/ERC4626.sol#L236-L238

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

    function _decimalsOffset() internal view virtual returns (uint8) {
        return 0;
    }

Proof of Concept

run in StakedUSDe.t.sol

  function testDonateDosStake() public {
    uint256 amount = 100 ether;
    // donate attack to make anyone unable to pass the minShare check
    address staker = alice;
    usdeToken.mint(address(stakedUSDe), 1 ether);
    usdeToken.mint(staker, amount);
    vm.startPrank(staker);
    usdeToken.approve(address(stakedUSDe), amount);
    vm.expectRevert(IStakedUSDe.MinSharesViolation.selector);
    stakedUSDe.deposit(amount, staker);
  }

Tools Used

Recommended Mitigation Steps

during constructor establish an initial conversion rate before shares get inflated.

Assessed type

Context

[L-1] silo can be made immutable.

Silo is deployed in constructor, and there is no setter in the StakedUSDeV2.

  constructor(IERC20 _asset, address initialRewarder, address owner) StakedUSDe(_asset, initialRewarder, owner) {
    silo = new USDeSilo(address(this), address(_asset));
    cooldownDuration = MAX_COOLDOWN_DURATION;
  }

[L-2] _deduplicateOrder returning a true and checks is redundant since it never returns a false valid from verifyNonce, which simply revert.

if (!_deduplicateOrder(order.benefactor, order.nonce)) revert Duplicate();

  function _deduplicateOrder(address sender, uint256 nonce) private returns (bool) {
    (bool valid, uint256 invalidatorSlot, uint256 invalidator, uint256 invalidatorBit) = verifyNonce(sender, nonce);
    mapping(uint256 => uint256) storage invalidatorStorage = _orderBitmaps[sender];
    invalidatorStorage[invalidatorSlot] = invalidator | invalidatorBit;
    return valid;
  }

  function verifyNonce(address sender, uint256 nonce) public view override returns (bool, uint256, uint256, uint256) {
    if (nonce == 0) revert InvalidNonce();
    uint256 invalidatorSlot = uint64(nonce) >> 8;
    uint256 invalidatorBit = 1 << uint8(nonce);
    mapping(uint256 => uint256) storage invalidatorStorage = _orderBitmaps[sender];
    uint256 invalidator = invalidatorStorage[invalidatorSlot];
    if (invalidator & invalidatorBit != 0) revert InvalidNonce();

    return (true, invalidatorSlot, invalidator, invalidatorBit);
  }