Ethena Labs - radev_sw's results

Enabling The Internet Bond

General Information

Platform: Code4rena

Start Date: 24/10/2023

Pot Size: $36,500 USDC

Total HM: 4

Participants: 147

Period: 6 days

Judge: 0xDjango

Id: 299

League: ETH

Ethena Labs

Findings Distribution

Researcher Performance

Rank: 44/147

Findings: 3

Award: $126.34

QA:
grade-b
Gas:
grade-b
Analysis:
grade-a

๐ŸŒŸ Selected for report: 1

๐Ÿš€ Solo Findings: 0

QA Report - Ethena Labs Audit Contest | 24 Oct 2023 - 30 Oct 2023


Executive summary

Overview

Project NameEthena Labs
Repositoryhttps://github.com/code-423n4/2023-10-ethena
WebsiteLink
TwitterLink
DocumentationLink
MethodsManual Review
Total nSLOC588 over 6 contracts

Scope

USDe.sol EthenaMinting.sol and the contract it extends, SingleAdminAccessControl.sol StakedUSDeV2.sol, the contract it extends, StakedUSDe.sol and the additional contract it creates USDeSilo.sol

ContractSLOCPurposeLibraries used
USDe.sol24USDe token stablecoin contract that grants another address the ability to mint USDe@openzeppelin/ERC20Burnable.sol @openzeppelin/ERC20Permit.sol @openzeppelin/Ownable2Step.sol
EthenaMinting.sol295The contract where minting and redemption occurs. USDe.sol grants this contract the ability to mint USDe@openzeppelin/ReentrancyGuard.sol
StakedUSDe.sol130Extension of ERC4626. Users stake USDe to receive stUSDe which increases in value as Ethena deposits protocol yield here@openzeppelin/ReentrancyGuard.sol @openzeppelin/ERC20Permit.sol @openzeppelin/ERC4626.sol
StakedUSDeV2.sol76Extends StakedUSDe, adds a redemption cooldown.
USDeSilo.sol20Contract to temporarily hold USDe during redemption cooldown
SingleAdminAccessControl.sol43EthenaMinting uses SingleAdminAccessControl rather than the standard AccessControl

Findings Summary

IDTitleSeverity
L-01The protocol does not ensure that the EthenaMinting.sol contract is the minter in the USDe contract, which can lead to impossible minting of USDe tokens for an indefinite period of timeLow
L-02The _computeDomainSeparator() function doesn't fully comply with the rules for EIP-712: DOMAIN_SEPARATORLow
L-03EIP712 has not been accurately applied to the Route structLow
L-04Malicious actor can evade the FULL_RESTRICTED_STAKER_ROLE roleLow
L-05getUnvestedAmount() is not needed to be read in StackedUSDe.sol#transferInRewards() function for the second timeLow
NC-01Don't grant DEFAULT_ADMIN_ROLE twice in the EthenaMinting contractNon Critical
NC-02Write modifier for if (msg.sender != minter) revert OnlyMinter(); check in the USDe#mint() function, so to be consistent with USDe#setMinter() function which performs the access control check in modifierNon Critical
NC-03Change the names of removeFromBlacklist() and addToBlacklist() functionsNon Critical
NC-04Failure to validate functions return values can result in errorsNon Critical
NC-05Add check in rescueTokens() if the to parameter is FULL_RESTRICTED_STAKER_ROLE and if revertNon Critical
NC-06The System Exclusively Accepts NoncesNon Critical
NC-07When adding an asset to supportedAssets, perform a check to ensure that the supported asset is not the NATIVE_TOKEN. If it is, then revertNon Critical
NC-08Don't assign role on msg.senderNon Critical
NC-09It is recommended to use deadline for meta-transactionsNon Critical
NC-10Lack of Event Emitting on Important for the Protocol functionsNon Critical

[L-01] The protocol does not ensure that the EthenaMinting.sol contract is the minter in the USDe contract, which can lead to a impossible minting of USDe tokens for an indefinite period

Summary:

The Ethena protocol consists of two contracts, "USDe" and "EthenaMinting." The "USDe" contract allows tokens to be minted only by the designated "minter" address, and this address can be set via the "setMinter" function. The "EthenaMinting" contract attempts to mint USDe tokens via the "mint" function of the "USDe" contract. However, the vulnerability is that the "EthenaMinting" contract has not been properly set as the minter in the "USDe" contract. In such a scenario, USDe tokens cannot be minted until the "EthenaMinting" contract is granted the minter role in the "USDe" contract.

  function mint(Order calldata order, Route calldata route, Signature calldata signature)
    external
    override
    nonReentrant
    onlyRole(MINTER_ROLE)
    belowMaxMintPerBlock(order.usde_amount)
  {
    if (order.order_type != OrderType.MINT) revert InvalidOrder();
    verifyOrder(order, signature);
    if (!verifyRoute(route, order.order_type)) revert InvalidRoute();
    if (!_deduplicateOrder(order.benefactor, order.nonce)) revert Duplicate();
    // Add to the minted amount in this block
    mintedPerBlock[block.number] += order.usde_amount;
    _transferCollateral(
      order.collateral_amount, order.collateral_asset, order.benefactor, route.addresses, route.ratios
    );
    usde.mint(order.beneficiary, order.usde_amount);
    emit Mint(
      msg.sender,
      order.benefactor,
      order.beneficiary,
      order.collateral_asset,
      order.collateral_amount,
      order.usde_amount
    );
  }
  function mint(address to, uint256 amount) external {
    if (msg.sender != minter) revert OnlyMinter();
    _mint(to, amount);
  }

In the contest page write:

USDe minter - can mint any amount of USDe tokens to any address. Expected to be the EthenaMinting contract

So it is essential to set the minter role in USDe.sol contract still in USDe constructor to EthenaMinting contract, otherwise after deploying the Ethena Protocol minting from EthenaMinting contract will be impossible.

Impact:

  • After deploying the Ethena Protocol minting from EthenaMinting contract will be impossible.
  • Disrupt the minting of USDe tokens for an indefinite period, causing a temporary halt in token minting. If the "EthenaMinting" contract is not set as the minter or if it loses the minter role in the "USDe" contract, users will be unable to mint USDe tokens, affecting the protocol's functionality and potentially causing inconvenience to users.

Recommendation Migration Steps:

  • In the constructor of USDe.sol contract set the minter to be the EthenaContract.sol contract.

So, implement the constructor of USDe.sol contract as follow:

    constructor(address admin, address ethenaMinting) ERC20("USDe", "USDe") ERC20Permit("USDe") {
        if (admin == address(0)) revert ZeroAddressException();
        _transferOwnership(admin);
        minter = ethenaMinting;
    }

[L-02] The _computeDomainSeparator() function doesn't fully comply with the rules for EIP-712: DOMAIN_SEPARATOR

GitHub Links:

Summary:

The _computeDomainSeparator() function doesn't fully comply with the rules for EIP-712: DOMAIN_SEPARATOR. As indicated in this article, in the EIP-712: DOMAIN_SEPARATOR section, it is mentioned that uint256 should be used instead of uint (same for int).

However, in the EthenaMinting.sol contract, the DOMAIN_SEPARATOR is generated as follows:

function _computeDomainSeparator() internal view returns (bytes32) {
    return keccak256(abi.encode(EIP712_DOMAIN, EIP_712_NAME, EIP712_REVISION, block.chainid, address(this)));
}

As we can see from solidity documentation, the global/special variable block.chainid is of type uint by default, not uint256. Consequently, the _computeDomainSeparator function doesn't explicitly cast block.chainid to uint256. This deviation means that the EthenaMinting contract doesn't fully adhere to the rules for EIP-712: DOMAIN_SEPARATOR.

Implement the _computeDomainSeparator as follows:

function _computeDomainSeparator() internal view returns (bytes32) {
    return keccak256(abi.encode(EIP712_DOMAIN, EIP_712_NAME, EIP712_REVISION, uint256(block.chainid), address(this)));
}

[L-03] EIP712 has not been accurately applied to the Route struct

GitHub Links:

Summary:

As defined by the EIP712 standard, array values should be encoded as the keccak256 hash of the concatenated encodeData of their contents. The current implementation in EthenaMinting.sol#encodeRoute() does not adhere to this rule for the Route Struct array fields.

Only, because the encodeRoute() function is not invoked in the protocol, can be safely removed.


[L-04] Malicious actor can evade the FULL_RESTRICTED_STAKER_ROLE role

GitHub Links:

Summary:

The protocol has introduced the FULL_RESTRICTED_STAKER_ROLE to empower the StakedUSDe owner with the authority to blacklist certain addresses and potentially manipulate their balances. However, this mechanism exhibits a vulnerability that could be bypassed with front funning attack.

  1. A user, either through malicious actions or being identified as a bad actor, comes under scrutiny.
  2. The StakedUSDe owner decides to blacklist the user's address and initiates a transaction by calling addToBlacklist() for that specific address.
  3. The user is vigilantly monitoring Ethereum's mempool and swiftly front-runs this transaction. As a result, the user rapidly transfers their entire stUSDe balance to another address that they control.
  4. While the user's original address is indeed blacklisted, they retain control over all their tokens, which are now held in the new address, and these tokens remain fully functional.

[L-05] getUnvestedAmount() is not needed to be read in StackedUSDe.sol#transferInRewards() function for the second time

GitHub Links:

Summary:

The StackedUSDe.sol#transferInRewards() function allows the owner to transfer rewards from the controller contract into this contract. However the function can be executed only if getUnvestedAmount() is greater than zero:

    if (getUnvestedAmount() > 0) revert StillVesting();

Later in the function newVestingAmount used "add specified amount from owner and getUnvestedAmount()". However getUnvestedAmount() will always be zero.

    /**
     * @notice Allows the owner to transfer rewards from the controller contract into this contract.
     * @param amount The amount of rewards to transfer.
     */
    function transferInRewards(uint256 amount) external nonReentrant onlyRole(REWARDER_ROLE) notZero(amount) {
        if (getUnvestedAmount() > 0) revert StillVesting();
        uint256 newVestingAmount = amount + getUnvestedAmount();

        vestingAmount = newVestingAmount;
        lastDistributionTimestamp = block.timestamp;
        // transfer assets from rewarder to this contract
        IERC20(asset()).safeTransferFrom(msg.sender, address(this), amount);

        emit RewardsReceived(amount, newVestingAmount);
    }

Rewrite the StackedUSDe.sol#transferInRewards() function as follow:

    /**
     * @notice Allows the owner to transfer rewards from the controller contract into this contract.
     * @param amount The amount of rewards to transfer.
     */
    function transferInRewards(uint256 amount) external nonReentrant onlyRole(REWARDER_ROLE) notZero(amount) {
        if (getUnvestedAmount() > 0) revert StillVesting();
    -   uint256 newVestingAmount = amount + getUnvestedAmount();

    -   vestingAmount = newVestingAmount;
        lastDistributionTimestamp = block.timestamp;
        // transfer assets from rewarder to this contract
        IERC20(asset()).safeTransferFrom(msg.sender, address(this), amount);

    -   emit RewardsReceived(amount, newVestingAmount);
    +   emit RewardsReceived(amount, amount);
    }

[NC-01] Don't grant DEFAULT_ADMIN_ROLE twice in the EthenaMinting contract

GitHub Links:

Summary:

One of the arguments in the constructor of the EthenaMinting contract is admin. In the constructor, it initially grants the DEFAULT_ADMIN_ROLE to msg.sender. Then, the code checks if msg.sender is different from the admin parameter, and if it is, the DEFAULT_ADMIN_ROLE is granted to the admin address. This logic is not efficient, and it is also inefficient for one contract to have two DEFAULT_ADMIN_ROLE addresses.

Mitigation:

  constructor(
    IUSDe _usde,
    address[] memory _assets,
    address[] memory _custodians,
    address _admin,
    uint256 _maxMintPerBlock,
    uint256 _maxRedeemPerBlock
  ) {
    if (address(_usde) == address(0)) revert InvalidUSDeAddress();
    if (_assets.length == 0) revert NoAssetsProvided();
-   if (_admin == address(0)) revert InvalidZeroAddress();
+   if (_admin == address(0) && msg.sender != admin) revert CUSTOM_ERROR();
    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]);
    }

    // Set the max mint/redeem limits per block
    _setMaxMintPerBlock(_maxMintPerBlock);
    _setMaxRedeemPerBlock(_maxRedeemPerBlock);

    if (msg.sender != _admin) {
      _grantRole(DEFAULT_ADMIN_ROLE, _admin);
    }

    _chainId = block.chainid;
    _domainSeparator = _computeDomainSeparator();

    emit USDeSet(address(_usde));
  }

[NC-02] Write modifier for if (msg.sender != minter) revert OnlyMinter(); check in the USDe#mint() function, so to be consistent with USDe#setMinter() function which performs the access control check in modifier

GitHub Links:

Code:

  function setMinter(address newMinter) external onlyOwner {
    emit MinterUpdated(newMinter, minter);
    minter = newMinter;
  }

  function mint(address to, uint256 amount) external {
    if (msg.sender != minter) revert OnlyMinter();
    _mint(to, amount);
  }

[NC-03] Change the names of removeFromBlacklist() and addToBlacklist() functions

GitHub Links:

Summary:

The functions is not properly named, because the roles that are granted/revoked are SOFT_RESTRICTED_STAKER_ROLE and FULL_RESTRICTED_STAKER_ROLE.


[NC-04] Failure to validate functions return values can result in errors

GitHub Links:

Summary:

Within the StakedUSDe contract, there are functions named addToBlacklist() and removeFromBlacklist() that utilize the _grantRole and _revokeRole methods respectively. It is essential to validate the return values of these functions in both scenarios. This ensures that when a role is granted, it confirms the target's absence of the role previously, and when a role is revoked, it confirms that the target indeed held this role.


[NC-05] Add check in rescueTokens() if the to parameter is FULL_RESTRICTED_STAKER_ROLE and if revert

GitHub Links:

Summary

FULL_RESTRICTED_STAKER_ROLE - address with this role can't burn his stUSDe tokens to unstake his USDe tokens, neither to transfer stUSDe tokens. His balance can be manipulated by the admin of StakedUSDe.

Every transaction that contains some sort of token transferring, revert if participant is someone with FULL_RESTRICTED_STAKER_ROLE, expect rescueTokens() function that transfer tokens accidentally sent to the contract. As the result owner can mistakenly transfer tokens to address with FULL_RESTRICTED_STAKER_ROLE thinking that transfer tokens accidentally sent to the contract.

Mitigation

Add check in rescueTokens() if the to parameter is FULL_RESTRICTED_STAKER_ROLE and if revert


[NC-06] The System Exclusively Accepts Nonces

GitHub Links:

Summary:

Description: Presently, the nonce system implemented in _deduplicateOrder() restricts us to having distinct slots for a user. Each slot accommodates bits. Nevertheless, in the _orderBitmaps mapping, both key and value types are uint256, which implies that distinct nonces could unintentionally intersect, even when they shouldn't (e.g., and).

Recommendation Migration Steps:

While it is improbable for nonces to reach such large values, we advise the team to address this concern, document the anticipated behavior in this situation, and implement any necessary corrections.


[NC-07] When adding an asset to supportedAssets, perform a check to ensure that the supported asset is not the NATIVE_TOKEN. If it is, then revert

GitHub Links:

Summary:

This will improve the code readability, because of the checks related to native tokens (ethers) performed in _transferCollateral() will be no needed.

  function _transferCollateral(
    uint256 amount,
    address asset,
    address benefactor,
    address[] calldata addresses,
    uint256[] calldata ratios
  ) internal {
    // cannot mint using unsupported asset or native ETH even if it is supported for redemptions
    if (!_supportedAssets.contains(asset) || asset == NATIVE_TOKEN) revert UnsupportedAsset();
    IERC20 token = IERC20(asset);
    uint256 totalTransferred = 0;
    for (uint256 i = 0; i < addresses.length; ++i) {
      uint256 amountToTransfer = (amount * ratios[i]) / 10_000;
      token.safeTransferFrom(benefactor, addresses[i], amountToTransfer);
      totalTransferred += amountToTransfer;
    }
    uint256 remainingBalance = amount - totalTransferred;
    if (remainingBalance > 0) {
      token.safeTransferFrom(benefactor, addresses[addresses.length - 1], remainingBalance);
    }
  }

[NC-08] Don't assign role on msg.sender

GitHub Links:

Summary

This a common smart contract security concern. In the specific context of Ethereum smart contracts, roles are used for access control: they designate which addresses have permission to perform certain actions, such as minting or burning tokens. When don't assign roles on msg.sender leads to a more secure system of role management to prevent the abuse of critical permissions in the smart contract environment. It's a call for stronger security measures and more thoughtful control distribution, especially for functions that can significantly impact the tokenomics or the participants' trust in the platform.

Overall - This is bad solution. Better use approach like in UniswapV3.


[NC-09] It is recommended to use deadline for meta-transactions

GitHub Links


[NC-10] Lack of Event Emitting on Important for the Protocol functions

Summary

The protocol currently lacks event emission in several crucial functions, which can hinder transparency and make tracking important actions challenging. Here are the instances where events are missing:

  1. USDe.sol#mint(): The mint() function in the USDe contract does not emit an event when new tokens are minted, making it difficult to track and verify minting activities.
  2. EthenaMinting#disableMintRedeem(): While the internal functions called within disableMintRedeem(), such as _setMaxMintPerBlock() and _setMaxRedeemPerBlock(), emit events for the new set values, the main goal of disableMintRedeem() differs. Adding an event specifically for disableMintRedeem would provide clarity and enhance visibility.
  3. StakedUSDe#_deposit(): The _deposit() function in the StakedUSDe contract lacks event emission, making it challenging to monitor deposit activities.
  4. StakedUSDe#rescueTokens(): The rescueTokens() function does not emit an event, potentially causing issues in tracking rescued tokens.
  5. removeFromBlacklist(), addToBlacklist(): These functions in the protocol lack event emission, making it difficult to track changes to the blacklist.
  6. For StakedUSDeV2: withdraw(), redeem(), unstake(): These functions do not emit events, which can hinder monitoring and auditing processes.

Adding appropriate events to these functions would greatly improve the protocol's transparency and auditability.


#1 - raymondfam

2023-11-02T01:54:17Z

False positives: NC-01, L-01, L-02 Pashov: L-03, L04, N-04 Bot: NC-10

#2 - c4-pre-sort

2023-11-02T01:54:24Z

raymondfam marked the issue as sufficient quality report

#3 - c4-judge

2023-11-14T17:09:05Z

fatherGoose1 marked the issue as grade-b

#4 - radeveth

2023-11-16T09:33:58Z

Hi, @raymondfam and @fatherGoose1! Thank you both for the quick judging.

I strongly believe that L-01 and NC-01 are valid findings.

For L-01, the protocol does not ensure that the EthenaMinting.sol contract has the minter role in the USDe contract. If you can show me where in the Solidity code this is happening, please provide the details.

Regarding NC-01, I believe that this should be classified as a non-critical severity vulnerability, as granting two DEFAULT_ADMIN_ROLE roles increases the centralization of the protocol twice.

#5 - radeveth

2023-11-22T16:56:15Z

Hello, @fatherGoose1!

Due to the comment above I think that my QA Report should be classified as A grade*.

Awards

6.4563 USDC - $6.46

Labels

bug
G (Gas Optimization)
grade-b
sufficient quality report
G-08

External Links

IDTitle
GAS-01Prefer abi.encodePacked() over abi.encode()
GAS-02Use bytes.concat() Instead of abi.encodePacked
GAS-03Emit Memory Values Instead of Storage Values
GAS-04Use assembly for math equations
GAS-05Use assembly to validate msg.sender
GAS-06Use the inputs/results of assignments rather than re-reading state variables
GAS-07x + y is more efficient than using += for state variables (likewise for -=)

[GAS-01] Prefer abi.encodePacked() over abi.encode()

For more information, refer to: Solidity Encode Gas Comparison

Issue Description: There is one instance of this issue in the EthenaMinting.sol contract:

๐Ÿ“ File: protocols/USDe/contracts/EthenaMinting.sol

49:   bytes32 private constant EIP712_DOMAIN_TYPEHASH = keccak256(abi.encodePacked(EIP712_DOMAIN)); 

49


[GAS-02] Use bytes.concat() Instead of abi.encodePacked

When concatenation is not used for hashing, bytes.concat is the preferred method as it is more gas-efficient.

Issue Description: There is one instance of this issue in the EthenaMinting.sol contract:

๐Ÿ“ File: protocols/USDe/contracts/EthenaMinting.sol

49:   bytes32 private constant EIP712_DOMAIN_TYPEHASH = keccak256(abi.encodePacked(EIP712_DOMAIN)); 

49

Recommended Mitigation Steps: Consider using bytes.concat() and upgrading to at least Solidity version 0.8.4 if required.


[GAS-03] Emit Memory Values Instead of Storage Values

Emit memory values instead of reading storage values again. This optimization can result in significant gas savings.

Gas Saved per Instance: ~97 (Total: ~582)

Issue Description: There are six instances of this issue:

๐Ÿ“ File: protocols/USDe/contracts/EthenaMinting.sol

439:     emit MaxMintPerBlockChanged(oldMaxMintPerBlock, maxMintPerBlock); 

446:     emit MaxRedeemPerBlockChanged(oldMaxRedeemPerBlock, maxRedeemPerBlock); 

439, 446

๐Ÿ“ File: protocols/USDe/contracts/SingleAdminAccessControl.sol

28:     emit AdminTransferRequested(_currentDefaultAdmin, newAdmin); 

74:     emit AdminTransferred(_currentDefaultAdmin, account); 

28, 74

๐Ÿ“ File: protocols/USDe/contracts/StakedUSDeV2.sol

133:     emit CooldownDurationUpdated(previousDuration, cooldownDuration); 

133

๐Ÿ“ File: protocols/USDe/contracts/USDe.sol

24:     emit MinterUpdated(newMinter, minter); 

24

Emitting memory values instead of reading storage values again can result in gas savings.

[GAS-04]: Use assembly for math equations

Replace (a * b) / c with the assembly equivalent div(mul(a, b), c) to save gas.

Example mitigation:
// Replace (a * b) / c with assembly
uint256 result;
assembly {
    result := div(mul(a, b), c)
}

[GAS-05]: Use assembly to validate msg.sender

You can use assembly to efficiently validate msg.sender. Here's an example of how to do this:

Example mitigation:
// Use assembly to validate msg.sender
assembly {
    if iszero(eq(msg.sender, _admin)) {
        revert(0, 0)
    }
}

[GAS-06]: Use the inputs/results of assignments rather than re-reading state variables

You should use the result of assignments rather than re-reading state variables. Here's an example:

Example mitigation:
// Use the result of the assignment instead of re-reading the state variable
uint256 newMaxMintPerBlock = maxMintPerBlock;
maxMintPerBlock = newMaxMintPerBlock + increment;
emit MaxMintPerBlockChanged(oldMaxMintPerBlock, maxMintPerBlock);

[GAS-07]: x + y is more efficient than using += for state variables (likewise for -=)

You should use x = x + y instead of x += y for state variables to save gas. Here's an example:

Example mitigation:
// Use x = x + y instead of x += y
mintedPerBlock[block.number] = mintedPerBlock[block.number] + order.usde_amount;

#0 - c4-pre-sort

2023-11-01T15:08:41Z

raymondfam marked the issue as sufficient quality report

#1 - c4-judge

2023-11-10T20:31:11Z

fatherGoose1 marked the issue as grade-b

Awards

115.3553 USDC - $115.36

Labels

analysis-advanced
grade-a
high quality report
selected for report
sponsor acknowledged
edited-by-warden
A-17

External Links

Ethena Labs Audit Contest Analysis Report | 24 Oct 2023 - 30 Oct 2023


#Topic
1Architecture Overview (Protocol Explanation, Codebase Explanation, Examples Scenarios of Intended Protocol Flow)
2Codebase Quality Analysis
3Centralization Risks
4Systemic Risks
5Attack Vectors Discussed During the Audit
6Example of Report that turned out to be invalid after I wrote a really good Explanation and PoC (The report explain the unstaking really well, so you can learn for it)

Pdf link -> https://www.docdroid.net/pwoydhp/analysis-report-pdf


1. Architecture Overview (Protocol Explanation, Codebase Explanation, Examples Scenarios of Intended Protocol Flow)

1.1. Protocol Explanation

  • Overview:

    Ethena is developing a DeFi ecosystem with a primary goal of offering a permissionless stablecoin, USDe, that allows users to earn yield within the system. This process is a contrast to traditional stablecoins like USDC, where the central authority (e.g., Circle) benefits from the yield. In Ethena's ecosystem, users can stake their USDe to earn stUSDe, which appreciates over time as the protocol generates yield.

  • Smart Contract Infrastructure:

    • USDe.sol: The contract for the USDe stablecoin, limited in functionality with controls for minting privileges.
    • EthenaMinting.sol: This contract mints and redeems USDe in a single, atomic, trustless transaction. Central to user interactions, handling minting and redemption of USDe. It employs EIP712 signatures for transactions, routing collateral through predefined safe channels, and includes security measures against potential compromises by limiting minting and providing emergency roles (GATEKEEPERS) to intervene in suspicious activities.
    • StakedUSDeV2.sol: Allows USDe holders to stake their tokens for stUSDe, earning yield from the protocol's profits. It incorporates mechanisms to prevent exploitation of yield payouts and has a cooldown period for unstaking. For legal compliance, it can restrict certain users (based on jurisdiction or law enforcement directives) from staking or freeze their assets, with the provision of fund recovery in extreme cases.
  • Roles in Ethena Ecosystem:

    • USDe minter - can mint any amount of USDe tokens to any address. Expected to be the EthenaMinting contract.
    • USDe owner - can set token minter and transfer ownership to another address
    • USDe token holder - can not just transfer tokens but burn them and sign permits for others to spend their balance
    • StakedUSDe admin - can rescue tokens from the contract and also to redistribute a fully restricted staker's stUSDe balance, as well as give roles to other addresses (for example the FULL_RESTRICTED_STAKER_ROLE role)
    • StakedUSDeV2 admin - has all power of "StakedUSDe admin" and can also call the setCooldownDuration method
    • REWARDER_ROLE - can transfer rewards into the StakedUSDe contract that will be vested over the next 8 hours
    • BLACKLIST_MANAGER_ROLE - can do/undo full or soft restriction on a holder of stUSDe
    • SOFT_RESTRICTED_STAKER_ROLE - address with this role can't stake his USDe tokens or get stUSDe tokens minted to him
    • FULL_RESTRICTED_STAKER_ROLE - address with this role can't burn his stUSDe tokens to unstake his USDe tokens, neither to transfer stUSDe tokens. His balance can be manipulated by the admin of StakedUSDe
    • MINTER_ROLE - can actually mint USDe tokens and also transfer EthenaMinting's token or ETH balance to a custodian address
    • REDEEMER_ROLE - can redeem collateral assets for burning USDe
    • EthenaMinting admin - can set the maxMint/maxRedeem amounts per block and add or remove supported collateral assets and custodian addresses, grant/revoke roles
    • GATEKEEPER_ROLE - can disable minting/redeeming of USDe and remove MINTER_ROLE and REDEEMER_ROLE roles from authorized accounts
  • What custodian is? (Chat GPT says)

    • In the realm of cryptocurrencies and blockchain technology, a "custodian" refers to an entity (company or smart contract) that holds and safeguards an individual's or institution's digital assets. The role of a custodian is critical in scenarios where security and proper asset management are paramount. This concept isn't exclusive to digital assets; traditional financial institutions have custodians as well.
    • Within a blockchain environment, an address usually refers to a specific destination where cryptocurrencies are sent. Think of it like an account number in the traditional banking sense. In the context of smart contracts or decentralized applications, custodianAddresses likely refer to the collection of blockchain addresses that are authorized to hold assets on behalf of others. These addresses are controlled by the custodian, which could be a smart contract or a third-party service that maintains the security of the private keys associated with these addresses.

1.2. Codebase Explanation & Examples Scenarios of Intended Protocol Flow

All possible Actions and Flows in Ethena Protocol:
1. Minting USDe:

Users provide stETH as collateral to mint USDe. The system gives an RFQ (Request for Quote) detailing how much USDe they can create. Upon agreement, the user signs a transaction, and Ethena mints USDe against the stETH, which is then employed in various yield-generating activities, primarily shorting ETH perpetual futures to maintain a delta-neutral position.

Additional Explanations Related to the Minting:

  • So, USDe will be equivalent to using DAI, USDC, USDT (USDe contract is just the ERC20 token for the stablecoin and this token will be the token used in StakedUSDeV2.sol staking contract) where it doesn't have any yield and only the holders of stUSDe will earn the generated yield.
  • The EthenaMinting.sol contract is the place where minting is done.
2. Yield Generating:

Ethena generates yield by taking advantage of the differences in staking returns (3-4% for stETH) and shorting ETH perpetuals (6-8%). Profits are funneled into an insurance fund and later distributed to stakers, enhancing the value of stUSDe relative to USDe.

3. Maintaining Delta Neutrality:

Ethena employs a strategy involving stETH and short positions in ETH perpetuals, ensuring the value stability of users' holdings against market volatility.

Example โ„– 1:

  1. Initial Setup: - The user initiates the process by sending 10 stETH to Ethena. The stETH is a token representing staked Ethereum in the Ethereum 2.0 network, allowing holders to earn rewards while keeping liquidity. At the time of the transaction, Ethereum's price is $2,000; thus, 10 stETH equals $20,000. - Ethena uses these 10 stETH to mint 20,000 USDe stablecoins for the user, reflecting the stETH's dollar value. - Simultaneously, Ethena opens a short position on Ethereum perpetual futures (ETH perps) equivalent to 10 ETH. Given the current Ethereum price of $2,000, this also represents a $20,000 position. This short position means that Ethena is betting on the Ethereum price going down.
  2. Market Movement and Its Impact: - Now, the market faces significant volatility, and the price of Ethereum drops by 90%. As a result, the value of the user's 10 stETH decreases to $2,000 (reflecting the 90% drop from the original $20,000 value). - However, because Ethena shorted 10 ETH worth of perps, the decrease in Ethereum's price is advantageous for this position. The short ETH perps position now has an unrealized profit of $18,000. This profit occurs because Ethena 'borrowed' the ETH at a higher price to open the position and can now 'buy' it back at a much lower price, pocketing the difference.
  3. Redemption Process: - The user decides to redeem their 20,000 USDe. For Ethena to honor this request, they need to provide the user with the equivalent value in stETH that the USDe represents. - Ethena closes the short position on the ETH perps, which means they 'buy' back the ETH at the current market price, realizing the $18,000 profit due to the price difference from when they opened the short position. - With the $18,000, Ethena purchases 90 stETH at the current market price ($200 per stETH, as the price has dropped by 90%). - Ethena then returns the original 10 stETH along with the 90 stETH purchased from the profits of the short position. So, the user receives 100 stETH, which, at the current market price, is worth $20,000.

Example โ„– 2:

  1. Initial Condition: - The price of ETH is $2,000. - The user sends in 10 stETH (equivalent to 10 ETH) to Ethena to mint 20,000 USDe (since 10 ETH at $2,000 per ETH is worth $20,000). - Ethena takes these 10 stETH and opens a short position on 10 ETH's worth of perpetual futures (perps) to hedge against the price movement of ETH.
  2. Market Movement: - The market goes up by 50%. Therefore, the price of ETH (and stETH, as itโ€™s pegged to the ETH value) increases to $3,000.
  3. Position Analysis: - The user's 10 stETH is now worth $30,000 due to the market increase. - However, Ethena's short position is now at a notional loss because it was betting on the price of ETH going down, not up. The loss on the short position is $10,000 (the increase in value per ETH is $1,000, and Ethena shorted 10 ETH).
  4. Redemption Process: - If the user decides to redeem their USDe, they will present their 20,000 USDe. - Considering the market movement, the short position's loss needs to be covered. Ethena has to close the short position and realize the loss of $10,000. - After covering the $10,000 loss, there's $20,000 worth of stETH left (approximately 6.67 stETH at the new rate of $3,000 per stETH) to return to the user.
  5. End Result: - The user initially had assets worth $20,000 (10 stETH). If they hadn't engaged with Ethena and simply held onto their 10 stETH, their assets would now be worth $30,000 due to the positive market movement. - By choosing to use Ethena's hedging mechanism, they've forfeited potential gains to safeguard against potential losses. They receive approximately 6.67 stETH (worth $20,000) back after the redemption process, missing out on the additional $10,000 value increase. - Essentially, the user's assets remained stable in USDe value, but they did not benefit from ETH's bullish market. Their asset value didnโ€™t decrease, but they also lost potential profit

2. Codebase Quality Analysis

  1. USDe.sol:

    usde-contract
    • Code Organization:

      • The contract is well-organized and follows the best practices for code layout and structure.
      • It uses OpenZeppelin contracts, which are widely recognized and audited.
      • The constructor initializes the contract and sets the owner/admin.
      • It provides functions to set the minter and mint USDe tokens.
    • Modifiers:

      • The contract uses the Ownable2Step modifier, which enforces two-step ownership transfer.
      • The onlyRole modifier is used to restrict certain functions to specific roles, enhancing security.
    • Minting:

      • The contract allows only the minter to mint new tokens, which is a good security measure.
      • It checks if the provided minter is valid.
    • Gas Efficiency:

      • The contract uses the SafeERC20 library for safe token transfers, ensuring protection against reentrancy attacks.
      • Gas-efficient practices are followed throughout the contract.
    • Overall, USDe.sol appears to be well-structured and follows best practices for security and code organization.

  1. EthenaMinting.sol:

    ethena-minting-contract
    • Code Organization:

      • It imports external libraries and contracts, including OpenZeppelin contracts.
      • The constructor initializes contract parameters and roles.
      • It contains functions for minting and redeeming USDe tokens.
    • Security Measures:

      • The contract enforces access control using role-based access control (RBAC) with different roles for minters, redeemers, and gatekeepers.
      • Gas limits for minting and redeeming are enforced to prevent abuse.
    • Signature Verification:

      • It verifies the signature of orders, ensuring that the orders are signed by authorized parties.
      • It uses EIP-712 for signature verification.
    • Gas Efficiency:

      • Gas-efficient practices are followed, and SafeERC20 is used for token transfers.
    • Domain Separator:

      • The contract computes the domain separator for EIP-712, enhancing security.
    • Deduplication:

      • The contract implements deduplication of taker orders to prevent replay attacks.
    • Supported Assets:

      • The contract maintains a list of supported assets.
    • Custodian Addresses:

      • It keeps track of custodian addresses and allows transfers to custodian wallets.
    • Overall, EthenaMinting.sol is well-structured, secure, and follows best practices for code organization and security.

  1. StakedUSDe.sol:

The contract inherits Implementation of the ERC4626 "Tokenized Vault Standard" from OZ

![erc4626-oz-contract](https://github.com/radeveth/Ethena-c4-Contest/blob/main/erc4626-oz-contract.png?raw=true) #### ![staked-usde-contract](https://github.com/radeveth/Ethena-c4-Contest/blob/main/staked-usde-contract.png?raw=true)
  • Code Organization:
    • It imports external libraries and contracts, including OpenZeppelin contracts.
    • The constructor initializes contract parameters and roles.
    • It contains functions for transferring rewards, managing the blacklist, rescuing tokens, and redistributing locked amounts.
    • Public functions are provided to query the total assets and unvested amounts.
    • The decimals function is overridden to return the number of decimal places.
    • Custom modifiers ensure input validation and role-based access control.
    • Hooks and functions are in place to enforce restrictions on specific roles and token transfers.
  1. StakedUSDeV2.sol:
staked-usde-v2-contract
  • Code Organization:
    • The contract extends StakedUSDe and inherits its code organization structure.
    • It introduces additional state variables, including cooldowns, silo, MAX_COOLDOWN_DURATION, and cooldownDuration.
    • Custom modifiers, ensureCooldownOff and ensureCooldownOn, are defined to control the execution of functions based on cooldown status.
    • The constructor initializes the contract and sets the cooldown duration.
    • External functions are provided for withdrawing, redeeming, and performing cooldown actions for assets and shares.
    • The contract enforces different behavior based on the cooldown duration, adhering to or breaking the ERC4626 standard.
    • The setCooldownDuration function allows the admin to update the cooldown duration.
  1. USDeSilo.sol:
usde-silo-contract
  • The contract is primary goal of USDeSilo.sol is to to hold the funds for the cooldown period whn user initiate unstaking.

General Observations:

  • Role-based access control is implemented for various functions, enhancing security.
  • Gas-efficient practices, such as using SafeERC20, are followed throughout the code.
  • The codebase of protocol includes comprehensive comments and region divisions for clarity.
  • Note that, The use of EIP-712 for signature verification adds an extra layer of security.
  1. SingleAdminAccessControl.sol:
single-admin-access-control-contract
  • EthenaMinting uses SingleAdminAccessControl rather than the standard AccessControl.

In summary, the Ethena Protocol's codebase appears to be of high quality, with a strong focus on security and code organization.


3. Centralization Risks

Actually, the Ethena Protocol contains many roles, each with quite a few abilities. This is necessary for the Protocol's logic and purpose.

The protocol assigns important roles like "MINTER," "REWARDER," and "ADMIN" to specific entities, potentially exposing the system to undue influence or risks if these roles are compromised.

So, these roles introduce several centralization risks. The most significant one is the scenario in which the MINTER role becomes compromised. An attacker/minter could then mint a billion USDe without collateral and dump them into pools, causing a black swan event that our insurance fund cannot cover.

However, Ethena addresses this problem by enforcing on-chain mint and redeem limitations of 100k USDe per block."

From the documentation:

Our solution is to enforce an on chain mint and redeem limitation of 100k USDe per block. In addition, we have GATEKEEPER roles with the ability to disable mint/redeems and remove MINTERS,REDEEMERS. GATEKEEPERS acts as a safety layer in case of compromised MINTER/REDEEMER. They will be run in seperate AWS accounts not tied to our organisation, constantly checking each transaction on chain and disable mint/redeems on detecting transactions at prices not in line with the market. In case compromised MINTERS or REDEEMERS after this security implementation, a hacker can at most mint 100k USDe for no collateral, and redeem all the collateral within the contract (we will hold ~$200k max), for a max loss of $300k in a single block, before GATEKEEPER disable mint and redeem. The $300k loss will not materialy affect our operations.

In summary, Ethena actually introduces several centralization risks due to the presence of many different roles in the Protocol. However, at the same time, the team has done its best to enforce measures that reduce the largest potential attack scenario to a maximum loss of $300k, which will not materially affect the Ethena operations/ecosystem.


4. Systemic Risks

Hereโ€™s an analysis of potential systemic

  1. Smart Contract Vulnerability Risk: Smart contracts can contain vulnerabilities that can be exploited by attackers. If a smart contract has critical security flaws, such as logic problems, this could lead to asset loss or system manipulation. I strongly recommend that, once the protocol is audited, necessary actions be taken to mitigate any issues identified by C4 Wardens
  1. Third-Party Dependency Risk: Contracts rely on external data sources, such as @openzeppelin/contracts-upgradeable, and there is a risk that if any issues are found with these dependencies in your contracts, the Ethena protocol could also be affected.

I observed that old versions of OpenZeppelin are used in the project, and these should be updated to the latest version:

  "name": "@openzeppelin/contracts-upgradeable",
  "description": "Secure Smart Contract library for Solidity",
  "version": "4.9.2",

The latest version is 4.9.3 (as of July 28, 2023), while the project uses version 4.9.2.


5. Attack Vectors Discussed During the Audit

  1. Issues related to Roles (Centralization Risks). Problems with roles changing.
  2. Breaking of Main Protocol Invariants
    • EthenaMinting.sol - User's signed EIP712 order, if executed, must always execute as signed. ie for mint orders, USDe is minted to user and collateral asset is removed from user based on the signed values.
    • Max mint per block should never be exceeded.
    • USDe.sol - Only the defined minter address can have the ability to mint USDe.
  3. DoS for important protocol functions/flows such as EtehnaMinting.sol#mint() (Minting Flow), EtehnaMinting.sol#redeem() (Redemption Flow), StakedUSDe#deposit()/StakedUSDe#_deposit() (Depositing Flow), StakedUSDeV2#unstake() (Unstaking Flow).
  4. Token transfer fails.
  5. Minting more than 100k USDe per block.
  6. Users cannot unstake/withdraw/redeem.
  7. Can users withdraw more than they actually are supposed to?
  8. Minting without providing collateral amount?

6. Example of report that turned out to be invalid after I wrote a really good Explanation and PoC (The report explain the unstaking really well, so you can learn for it)

REPORT:


Title: Users will not be able to withdraw/redeem their assets/shares from StackedUSDeV2 contract. (Inefficient logic)

Explanation

StakedUSDeV2.sol is where holders of USDe stablecoin can stake their stablecoin, get stUSDe in return and earn yield. The Etehna Protocol yield is paid out by having a REWARDER role of the staking contract send yield in USDe, increasing the stUSDe value with respect to USDe. This contract is a modification of the ERC4626 standard, with a change to vest in rewards linearly over 8 hours

When the unstake process is initiated, from the user's perspective, stUSDe is burnt immediately, and they will be able to invoke the withdraw() function after cooldown is up to get their USDe in return. Behind the scenes, on burning of stUSDe, USDe is sent to a seperate USDeSilo contract to hold the funds for the cooldown period. And on withdrawal, the staking contract moves user funds from USDeSilo contract out to the user's address.

  1. Functions respond for redemption, starting of cooldown and transferring of converted underlying asset to USSDeSilo contract are cooldownAssets() and cooldownShares()
  function cooldownAssets(uint256 assets, address owner) external ensureCooldownOn returns (uint256) {
    if (assets > maxWithdraw(owner)) revert ExcessiveWithdrawAmount();

    uint256 shares = previewWithdraw(assets);

    cooldowns[owner].cooldownEnd = uint104(block.timestamp) + cooldownDuration;
    cooldowns[owner].underlyingAmount += assets;

    _withdraw(_msgSender(), address(silo), owner, assets, shares);

    return shares;
  }

  /// @notice redeem shares into assets and starts a cooldown to claim the converted underlying asset
  /// @param shares shares to redeem
  /// @param owner address to redeem and start cooldown, owner must allowed caller to perform this action
  function cooldownShares(uint256 shares, address owner) external ensureCooldownOn returns (uint256) {
    if (shares > maxRedeem(owner)) revert ExcessiveRedeemAmount();

    uint256 assets = previewRedeem(shares);

    cooldowns[owner].cooldownEnd = uint104(block.timestamp) + cooldownDuration;
    cooldowns[owner].underlyingAmount += assets;

    _withdraw(_msgSender(), address(silo), owner, assets, shares);

    return assets;
  }
  1. After cooldown is finished, user can call unstake() to claim the staking amount after.
  function unstake(address receiver) external {
    UserCooldown storage userCooldown = cooldowns[msg.sender];
    uint256 assets = userCooldown.underlyingAmount;

    if (block.timestamp >= userCooldown.cooldownEnd) {
      userCooldown.cooldownEnd = 0;
      userCooldown.underlyingAmount = 0;

      silo.withdraw(receiver, assets);
    } else {
      revert InvalidCooldown();
    }
  }

The function check if the cooldown is finished by block.timestamp >= userCooldown.cooldownEnd check. REMEMBER: The assets are transferred from USDeSilo contract -> https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/USDeSilo.sol#L28-L30

Described logic above work only if the cooldown is set to number greater than zero. (aka the cooldown is active)

The Ethena can decide to disable the cooldown period, so the users to be able unstake without cooldown period. If this is done, the user will be able to call directly withdraw()/redeem() to unstake:

  function withdraw(uint256 assets, address receiver, address owner)
    public
    virtual
    override
    ensureCooldownOff
    returns (uint256)
  {
    return super.withdraw(assets, receiver, owner);
  }

  /**
   * @dev See {IERC4626-redeem}.
   */
  function redeem(uint256 shares, address receiver, address owner)
    public
    virtual
    override
    ensureCooldownOff
    returns (uint256)
  {
    return super.redeem(shares, receiver, owner);
  }

REMEMBER: The assets are transferred from StakedUSDeV2 contract -> https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/USDeSilo.sol#L28-L30

Proof of Concept

So, the problem in Ethena protocol logic aries exactly when Ethena decide to disable the cooldown period. Lets illustrate the following example:

Scenario

  • Cooldown Period: 50 days
  1. Alice deposit her 10000 USDe tokens in StakedUSDeV2 contract.
  2. Bob also deposit his 20000 USDe tokens in StakedUSDeV2 contract.
  3. After some time Bob decide to unstake and call cooldownAssets() to start of cooldown period and converted underlying asset are transferred to USSDeSilo contract and he redeem his 20000 USDe tokens.
  4. After 30 days Ethena disable cooldown period.
  5. Now users are supposed to unstake directly from withdraw()/redeem().
  6. But when the Bob tries to call withdraw()/redeem() he will not be able to get his converted underlying asset, because they are in USDeSilo contract.
  7. Alice call withdraw()/redeem() and get his converted underlying asset.

Tools Used

  • Manual Inspection

After all, I observed that users who have already called cooldownAssets()/cooldownShares() can call the unstake() function again to retrieve their converted underlying asset.


Time spent:

35 hours

#0 - c4-pre-sort

2023-11-01T14:54:08Z

raymondfam marked the issue as high quality report

#1 - c4-sponsor

2023-11-09T19:58:31Z

FJ-Riveros (sponsor) acknowledged

#2 - c4-judge

2023-11-10T19:34:15Z

fatherGoose1 marked the issue as grade-a

#3 - c4-judge

2023-11-14T17:26:59Z

fatherGoose1 marked the issue as selected for report

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax ยฉ 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter