Ethena Labs - ast3ros's results

Lines of code

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

Vulnerability details

Impact

The current _withdraw function implementation allows soft-restricted stakers to withdraw stUSDe in exchange for USDe, effectively earning yield. This is a legal violation.

Proof of Concept

Legally, a soft-restricted staker is prohibited from certain activities, including depositing USDe to obtain stUSDe or withdrawing stUSDe for USDe.

    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. However they can participate in earning yield by buying and selling stUSDe on the open market.

https://github.com/code-423n4/2023-10-ethena/blob/ee67d9b542642c9757a6b826c82d0cae60256509/README.md#L98

Upon examining the _withdraw function in the StakedUSDe contract, it appears that only the FULL_RESTRICTED_STAKER_ROLE is blocked from withdrawal, leaving soft-restricted stakers unchecked. There can be a case when a user is being soft restricted after he is deposited into the StakedUSDe (his ccountry is included in the blocked list), he can still withdraw and earn yield.

This creates a legal vulnerability for the Ethena protocol, which is obligated to restrict yield payments to users from certain jurisdictions. Currently, there's no on-chain mechanism to enforce this requirement.

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

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

Tools Used

Manual

Recommended Mitigation Steps

When withdrawing, check if user is soft restricted:

• Disallow the withdrawal operation
• Alternatively, only permit the withdrawal of the original deposited amount, excluding any earned yield.

Assessed type

Other

[NC-1] Custodian length can be zero when EthenaMinting is created.

The _custodians array is not checked for zero length in constructor. If there is no custodian set, functionality of the EthenalMinting such as mint can be blocked.

    if (address(_usde) == address(0)) revert InvalidUSDeAddress();
    if (_assets.length == 0) revert NoAssetsProvided();
    if (_admin == address(0)) revert InvalidZeroAddress();
    usde = _usde;

    _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);

    for (uint256 i = 0; i < _assets.length; i++) {
        addSupportedAsset(_assets[i]);
    }

    for (uint256 j = 0; j < _custodians.length; j++) {
        addCustodianAddress(_custodians[j]);
    }

https://github.com/code-423n4/2023-10-ethena/blob/de36fe47c5d2c47e739b8e07fc69bd24f28e5762/contracts/EthenaMinting.sol#L119-L132

To address this issue, add checking length for _custodians array.

    if (_custodians.length == 0) revert NoAssetsProvided();

[NC-2] Unnecessary calculating getUnvestedAmount() in newVestingAmount

The getUnvestedAmount is unnecessary in calculating newVestingAmount since getUnvestedAmount is equal 0.

    if (getUnvestedAmount() > 0) revert StillVesting();
    uint256 newVestingAmount = amount + getUnvestedAmount();

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