Ethena Labs - KIntern_NA's results

Lines of code

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

Vulnerability details

Impact

FULL_RESTRICTED_STAKER_ROLE is still able to withdraw StakedUSDe tokens by approving to another address. It results in protocol has a risk of being unable to manipulate the funds of blacklisted address (by redistributeLockedAmount function).

Proof of concept

First, protocol intends to prevent the FULL_RESTRICTED_STAKER_ROLE address from being able to withdraw StakedUSDe tokens:

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

However, this function doesn't check the role of the _owner address. StakedUSDe inherits the ERC4626 contract, which allows users to withdraw on behalf of another address by using their allowance. https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/extensions/ERC4626.sol#L260-L269

/**
 * @dev Withdraw/redeem common workflow.
 */
function _withdraw(
    address caller,
    address receiver,
    address owner,
    uint256 assets,
    uint256 shares
) internal virtual {
    if (caller != owner) {
        _spendAllowance(owner, caller, shares);
    }
    ...

Therefore, the FULL_RESTRICTED_STAKER_ROLE can approve another address and use that address to withdraw StakedUSDe. This action leads the protocol to be at risk of being unable to manipulate the funds of blacklisted addresses.

For example, Alice is an attacker who deposited malicious/attacked funds, so the protocol blacklists her address. Subsequently, Ethena's admin wants to redistribute Alice's StakedUSDe tokens. However, Alice promptly approves a new address and uses that new address to withdraw StakedUSDe before the admin's transaction. Hence, Alice can escape with her funds, causing harm to the protocol.

Tools Used

Manual Review

Recommended Mitigation Steps

Should add the check for the role of _owner address in _withdraw function:

if (hasRole(FULL_RESTRICTED_STAKER_ROLE, caller) || 
    hasRole(FULL_RESTRICTED_STAKER_ROLE, receiver) || 
    hasRole(FULL_RESTRICTED_STAKER_ROLE, _owner)) {
      revert OperationNotAllowed();
    }

Assessed type

Access Control