Ethena Labs - JCK'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: 69/147

Findings: 3

Award: $25.22

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

🌟 Selected for report: 0

🚀 Solo Findings: 0

LOW Risk

NumberIssueInstances
[L-01]For loops in public or external functions should be avoided due to high gas costs and possible DOS3
[L-02]Use descriptive constant instead of 0 as a parameter2
[L-03]Revert on transfer to the zero address4
[L-04]Functions calling contracts with transfer hooks are missing reentrancy guards3
[L-05]Lack of User Validation in the EthenaMinting.sol2
[L-06]Lack of Limitations in the removeSupportedAsset Function1
[L-07]When removing _supportedAssets _supportedAssets.remove(asset) should be set to 0 for inactive1

[L‑01] For loops in public or external functions should be avoided due to high gas costs and possible DOS

In Solidity, for loops can potentially cause Denial of Service (DoS) attacks if not handled carefully. DoS attacks can occur when an attacker intentionally exploits the gas cost of a function, causing it to run out of gas or making it too expensive for other users to call. Below are some scenarios where for loops can lead to DoS attacks: Nested for loops can become exceptionally gas expensive and should be used sparingly

There are 3 instances of this issue:

file: main/contracts/EthenaMinting.sol

126   for (uint256 i = 0; i < _assets.length; i++) {
            addSupportedAsset(_assets[i]);
        }
    
130   for (uint256 j = 0; j < _custodians.length; j++) {
            addCustodianAddress(_custodians[j]);
        }

424   for (uint256 i = 0; i < addresses.length; ++i) {
            uint256 amountToTransfer = (amount * ratios[i]) / 10_000;
            token.safeTransferFrom(benefactor, addresses[i], amountToTransfer);
            totalTransferred += amountToTransfer;
        }

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L126-L128

[L‑02] Use descriptive constant instead of 0 as a parameter

Passing 0 or 0x0 as a function argument can sometimes result in a security issue(e.g. passing zero as the slippage parameter). A historical example is the infamous 0x0 address bug where numerous tokens were lost. This happens because 0 can be interpreted as an uninitialized address, leading to transfers to the 0x0 address, effectively burning tokens. Moreover, 0 as a denominator in division operations would cause a runtime exception. It's also often indicative of a logical error in the caller's code.

Consider using a constant variable with a descriptive name, so it's clear that the argument is intentionally being used, and for the right reasons..

file:  main/contracts/EthenaMinting.sol

230    _setMaxMintPerBlock(0);

231    _setMaxRedeemPerBlock(0);

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L230

[L‑03] Revert on transfer to the zero address

It's good practice to revert a token transfer transaction if the recipient's address is the zero address. This can prevent unintentional transfers to the zero address due to accidental operations or programming errors. Many token contracts implement such a safeguard, such as

file:  main/contracts/EthenaMinting.sol

426   token.safeTransferFrom(benefactor, addresses[i], amountToTransfer);

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L426

file: main/contracts/StakedUSDe.sol

96   IERC20(asset()).safeTransferFrom(msg.sender, address(this), amount);

140  IERC20(token).safeTransfer(to, amount);

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

file: main/contracts/USDeSilo.sol

29   USDE.transfer(to, amount);

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/USDeSilo.sol#L29

[L-04] Functions calling contracts with transfer hooks are missing reentrancy guards

Even if the function follows the best practice of check-effects-interaction, not using a reentrancy guard when there may be transfer hooks will open the users of this protocol up to read-only reentrancies with no way to protect against it, except by block-listing the whole protocol.

file: main/contracts/EthenaMinting.sol

408   IERC20(asset).safeTransfer(beneficiary, amount);

426  token.safeTransferFrom(benefactor, addresses[i], amountToTransfer);

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L408

file: main/contracts/StakedUSDe.sol

140    IERC20(token).safeTransfer(to, amount);

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

[L-05] Lack of User Validation in the EthenaMinting.sol

Vulnerability details

the setDelegatedSigner function lacks crucial checks to verify the validity and authenticity of the user attempting to delegate authority to another address.

Unauthorized Delegation:

Users, denoted by msg.sender, can freely invoke the setDelegatedSigner function to designate any Ethereum address as a delegated signer without undergoing any validation or authentication. This absence of validation exposes the system to the possibility of unauthorized delegation, deviating from the intended security model. Delegating signing authority to an unverified or unauthorized address introduces security vulnerabilities. Malicious actors may exploit this deficiency to impersonate authorized signers or engage in fraudulent activities, potentially jeopardizing the overall security of the system.

Loss of Control:

The absence of proper user validation deprives the contract owner or administrators of effective control over the delegation of signing authority. This lack of control can result in a disorderly and potentially chaotic delegation system.

file:  main/contracts/EthenaMinting.sol

235    function setDelegatedSigner(address _delegateTo) external {
    delegatedSigner[_delegateTo][msg.sender] = true;
    emit DelegatedSignerAdded(_delegateTo, msg.sender);
  }

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

Impact of the Vulnerability: compromised system integrity, a heightened potential for fraudulent activities, and a lack of accountability in the delegation process. Unauthorized delegations may disrupt the system's expected behavior and security, potentially leading to financial or reputational damage.

Proof of Concept: To address these Vulnerabilitys, it is strongly recommended to enhance the setDelegatedSigner function with robust user authentication and access control mechanisms. These measures could include requiring users to provide authentication, such as a valid signature and ensuring that they possess the necessary privileges to delegate signing authority. Access control can be implemented through role-based permissions or other appropriate methods, enabling better control and oversight. Additionally, the introduction of an approval process or a whitelist for delegated signers can fortify the security and governance of the delegation system, mitigating the identified Vulnerabilitys.

in unstake() it's essential to clarify who the staker is, how assets are obtained for staking, and implement appropriate access control mechanisms for the withdraw function. Access control can be enforced through role-based permissions or other authentication methods to ensure that only authorized users can execute withdrawal operations. Additionally, proper documentation and comments in the code should clarify the intended behavior of the unstake function and the associated withdraw function to avoid misunderstandings and ensure secure operation.

file: main/contracts/StakedUSDeV2.sol

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

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/StakedUSDeV2.sol#L78-L90

[L-06] Lack of Limitations in the removeSupportedAsset Function

Vulnerability details:

The removeSupportedAsset function, as presented, lacks any limitations or checks on the quantity of assets that can be removed. This omission introduces a substantial Vulnerability: users with the DEFAULT_ADMIN_ROLE can potentially remove numerous assets simultaneously or even remove all supported assets. The associated Vulnerability are as follows:

Mass Removal of Assets: Users endowed with the DEFAULT_ADMIN_ROLE have the capability to initiate the removal of a considerable number of assets in a single transaction. This creates the Vulnerability of a massive asset removal event, potentially disrupting the system and adversely affecting users who rely on these assets.

Loss of Functionality: In the event that all supported assets are removed, the system may suffer a profound loss of functionality. Such a scenario could render the system unusable and lead to severe inconvenience for its users, resulting in significant disruptions.

Lack of Control: The absence of limitations or checks within the function exacerbates the Vulnerability of a loss of control and oversight over asset removals. This lack of control can lead to unintended consequences, disputes, and disruptions within the ecosystem.

file: contracts/EthenaMinting.sol

259  function removeSupportedAsset(address asset) external onlyRole(DEFAULT_ADMIN_ROLE) {
    if (!_supportedAssets.remove(asset)) revert InvalidAssetAddress();
    emit AssetRemoved(asset);
  }

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L259-L262

Impact of the Vulnerability: loss of key functionality, and potential disputes or user dissatisfaction.

Proof of Concept: To address these Vulnerability, it is highly recommended to implement appropriate limitations and controls on asset removal within the removeSupportedAsset function. This may involve setting a maximum limit on the number of assets that can be removed in a single transaction or over a specific timeframe. Additionally, consider the implementation of governance mechanisms or the requirement of multi-signature approvals for asset removals. These measures are essential to ensure that asset removals are carried out in a controlled and deliberate manner, thus reducing the Vulnerability of mass or unauthorized removals and safeguarding the system's stability and functionality.

[L-07] When removing _supportedAssets _supportedAssets.remove(asset) should be set to 0 for inactive

Function removeSupportedAsset() removes a _supportedAssets if _supportedAssets.remove(asset) however it does not set _supportedAssets.remove of the _supportedAssets to 0 for inactive. In EnumerableSet.AddressSet state it says if a _supportedAssets is inactive activationRound should be 0.

file: main/contracts/EthenaMinting.sol

259   function removeSupportedAsset(address asset) external onlyRole(DEFAULT_ADMIN_ROLE) {
    if (!_supportedAssets.remove(asset)) revert InvalidAssetAddress();
    emit AssetRemoved(asset);
  } 

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L259-L261

#0 - c4-pre-sort

2023-11-02T02:16:23Z

raymondfam marked the issue as sufficient quality report

#1 - c4-judge

2023-11-14T17:07:02Z

fatherGoose1 marked the issue as grade-b

Awards

6.4563 USDC - $6.46

Labels

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

External Links

Gas Optimizations

NumberIssueInstancesTotal gas saved
[G-01]Possible Optimization EthenaMinting.sol2
[G-02]Possible Optimization in EthenaMinting.sol2
[G-03]Possible Optimization in verifyNonce() function1
[G-04]Optimize the function redistributeLockedAmount()1
[G-05]Optimize check order: avoid making any state reads/writes if we have to validate some function parameters1
[G-06]public functions not called by the contract should be declared external instead8
[G-07]Amounts should be checked for 0 before calling a transfer3
[G-08]x += y costs more gas than x = x + y for state variables618
[G-09]Do not initialize variables with default values1030
[G-10]Cache external calls outside of loop to avoid re-calling function on each iteration1
[G-11]Use assembly to perform efficient back-to-back calls2
[G-12]Use calldata instead of memory for function arguments that do not get mutated1
[G-13]Use hardcoded address instead of address(this)5
[G-14]Use uint256(1)/uint256(2) instead for true and false boolean states20200000
[G-15]Use assembly to validate msg.sender515
[G-16]A modifier used only once and not being inherited should be inlined to save gas2
[G-17]Use assembly for loops3
[G-18]Should use arguments instead of state variable3
[G-19]Can make the variable outside the loop to save gas1731
[G-20]abi.encode() is less efficient than abi.encodepacked()3153951
[G-21]Using delete statement can save gas1
[G-22]Use Modifiers Instead of Functions To Save Gas3
[G-23]Gas saving is achieved by removing the delete keyword (~60k)1
[G-24]Counting down in for statements is more gas efficient450868

[G-01] Possible Optimization EthenaMinting.sol

In the mint function, there are multiple calls of Order calldata order. This could be optimized by declaring it once at the beginning of the function and reusing the reference. This would save the gas used by the function call.

file: contracts/EthenaMinting.sol

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

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L162-L187

[PASS] testDelegateSuccessfulMint() (gas: 241444) [PASS] testDelegateSuccessfulMint() (gas: 240800)

same issue in redeem() function

file: contracts/EthenaMinting.sol

194   function redeem(Order calldata order, Signature calldata signature)
    external
    override
    nonReentrant
    onlyRole(REDEEMER_ROLE)
    belowMaxRedeemPerBlock(order.usde_amount)
  {
    if (order.order_type != OrderType.REDEEM) revert InvalidOrder();
    verifyOrder(order, signature);
    if (!_deduplicateOrder(order.benefactor, order.nonce)) revert Duplicate();
    // Add to the redeemed amount in this block
    redeemedPerBlock[block.number] += order.usde_amount;
    usde.burnFrom(order.benefactor, order.usde_amount);
    _transferToBeneficiary(order.beneficiary, order.collateral_asset, order.collateral_amount);
    emit Redeem(
      msg.sender,
      order.benefactor,
      order.beneficiary,
      order.collateral_asset,
      order.collateral_amount,
      order.usde_amount
    );
  }

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

[PASS] testDelegateSuccessfulRedeem() (gas: 325215) [PASS] testDelegateSuccessfulRedeem() (gas: 324776)

Same issue in verifyOrder() function

file: contracts/EthenaMinting.sol

339     function verifyOrder(Order calldata order, Signature calldata signature) public view override returns (bool, bytes32) {
    bytes32 taker_order_hash = hashOrder(order);
    address signer = ECDSA.recover(taker_order_hash, signature.signature_bytes);
    if (!(signer == order.benefactor || delegatedSigner[signer][order.benefactor])) revert InvalidSignature();
    if (order.beneficiary == address(0)) revert InvalidAmount();
    if (order.collateral_amount == 0) revert InvalidAmount();
    if (order.usde_amount == 0) revert InvalidAmount();
    if (block.timestamp > order.expiry) revert SignatureExpired();
    return (true, taker_order_hash);
  }

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L339-L348

recomondation


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

[G-02] Possible Optimization in EthenaMinting.sol

The verifyOrder functions check the order.collateral_amount and order.usde_amount with zero. instead of this checking you can checke order.collateral_amount and order.usde_amount with eache other can save gas because of removing of one if stamtent

file: main/contracts/EthenaMinting.sol

339   function verifyOrder(Order calldata order, Signature calldata signature) public view override returns (bool, bytes32) {
      bytes32 taker_order_hash = hashOrder(order);
      address signer = ECDSA.recover(taker_order_hash, signature.signature_bytes);
      if (!(signer == order.benefactor || delegatedSigner[signer][order.benefactor])) revert InvalidSignature();
      if (order.beneficiary == address(0)) revert InvalidAmount();
      if (order.collateral_amount == 0) revert InvalidAmount();
      if (order.usde_amount == 0) revert InvalidAmount();
      if (block.timestamp > order.expiry) revert SignatureExpired();
      return (true, taker_order_hash);
    }

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L339-L348

Gas report

[PASS] testCanUndelegate() (gas: 117886) [PASS] testDelegateFailureMint() (gas: 111434) [PASS] testDelegateFailureRedeem() (gas: 286158) [PASS] testDelegateSuccessfulMint() (gas: 241444) [PASS] testDelegateSuccessfulRedeem() (gas: 325215)

[PASS] testCanUndelegate() (gas: 117886) [PASS] testDelegateFailureMint() (gas: 111434) [PASS] testDelegateFailureRedeem() (gas: 286135) [PASS] testDelegateSuccessfulMint() (gas: 241421) [PASS] testDelegateSuccessfulRedeem() (gas: 325178)

The constructor check the address(_usde) and _admin addresses. to address(0) if you ckeck address(_usde) and _admin with each other instead of ckecking them to address(0) you can remove one if statment and can save gas

file: contracts/EthenaMinting.sol

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

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

[G-03] Possible Optimization in verifyNonce() function

In the verifyNonce function, the contract performs a number of operations after checking if the invalidator & invalidatorBit in not zero. If the invalidator & invalidatorBit is not zero, these operations are unnecessary. By using a return statement after the revert, we can avoid these unnecessary operations.

file: contracts/EthenaMinting.sol

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

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L377-L386

[PASS] testCanUndelegate() (gas: 117886) [PASS] testDelegateFailureMint() (gas: 111434) [PASS] testDelegateFailureRedeem() (gas: 286158) [PASS] testDelegateSuccessfulMint() (gas: 241444) [PASS] testDelegateSuccessfulRedeem() (gas: 325215)

[PASS] testCanUndelegate() (gas: 117886) [PASS] testDelegateFailureMint() (gas: 111434) [PASS] testDelegateFailureRedeem() (gas: 98832) [PASS] testDelegateSuccessfulMint() (gas: 136217) [PASS] testDelegateSuccessfulRedeem() (gas: 98813)

[G-04] Optimize the function redistributeLockedAmount()

in function redistributeLockedAmount() berfor to execute _burn() first check the if (to != address(0)) _mint(to, amountToDistribute); condation because only the // to address of address(0) enables burning to save gas

file: contracts/StakedUSDe.sol

148    function redistributeLockedAmount(address from, address to) external onlyRole(DEFAULT_ADMIN_ROLE) {
    if (hasRole(FULL_RESTRICTED_STAKER_ROLE, from) && !hasRole(FULL_RESTRICTED_STAKER_ROLE, to)) {
      uint256 amountToDistribute = balanceOf(from);
      _burn(from, amountToDistribute);
      // to address of address(0) enables burning
      if (to != address(0)) _mint(to, amountToDistribute);

      emit LockedAmountRedistributed(from, to, amountToDistribute);
    } else {
      revert OperationNotAllowed();
    }
  }

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

berfore: [PASS] test_redistributeLockedAmount() (gas: 226215) After: [PASS] test_redistributeLockedAmount() (gas: 210297)

[G-05] Optimize check order: avoid making any state reads/writes if we have to validate some function parameters

file: contracts/EthenaMinting.sol

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

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L111-L146

We have a check for function parameters if msg.sender != _admin. If we look at our constructor, the operations that happen before this check consume so much gas, as they involve reading and writing states. first check this condation after chaking do other operations.

[G-06] public functions not called by the contract should be declared external instead

when a function is declared as public, it is generated with an internal and an external interface. This means the function can be called both internally (within the contract) and externally (by other contracts or accounts). However, if a public function is never called internally and is only expected to be invoked externally, it is more gas-efficient to explicitly declare it as external.

file: contracts/EthenaMinting.sol

334  function encodeRoute(Route calldata route) public pure returns (bytes memory) {

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L334

file: contracts/SingleAdminAccessControl.sol

41   function grantRole(bytes32 role, address account) public override onlyRole(DEFAULT_ADMIN_ROLE) notAdmin(role) 

50   function revokeRole(bytes32 role, address account) public override onlyRole(DEFAULT_ADMIN_ROLE) notAdmin(role) {

65   function owner() public view virtual returns (address) {

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/SingleAdminAccessControl.sol#L41

file: contracts/StakedUSDe.sol

166   function totalAssets() public view override returns (uint256) {

184   function decimals() public pure override(ERC4626, ERC20) returns (uint8) {

257   function renounceRole(bytes32, address) public virtual override {

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

file: contracts/USDe.sol

33    function renounceOwnership() public view override onlyOwner {

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/USDe.sol#L33

[G-07] Amounts should be checked for 0 before calling a transfer

It can be beneficial to check if an amount is zero before invoking a transfer function, such as transfer or safeTransfer, to avoid unnecessary gas costs associated with executing the transfer operation. If the amount is zero, the transfer operation would have no effect, and performing the check can prevent unnecessary gas consumption.

file:  contracts/EthenaMinting.sol   

253   IERC20(asset).safeTransfer(wallet, amount);

408   IERC20(asset).safeTransfer(beneficiary, amount);

426   token.safeTransferFrom(benefactor, addresses[i], amountToTransfer);

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L253

[G-08] x += y costs more gas than x = x + y for state variables

file: contracts/EthenaMinting.sol
 
174   mintedPerBlock[block.number] += order.usde_amount;

205   redeemedPerBlock[block.number] += order.usde_amount;

368   totalRatio += route.ratios[i];

427   totalTransferred += amountToTransfer;

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L174

file: contracts/StakedUSDeV2.sol

101   cooldowns[owner].underlyingAmount += assets;

117   cooldowns[owner].underlyingAmount += assets;

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/StakedUSDeV2.sol#L101

[G-09] Do not initialize variables with default values

file: contracts/EthenaMinting.sol

236   delegatedSigner[_delegateTo][msg.sender] = true;

242   delegatedSigner[_removedSigner][msg.sender] = false;

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L236

file: contracts/EthenaMinting.sol

126   for (uint256 i = 0; i < _assets.length; i++) {

130   for (uint256 j = 0; j < _custodians.length; j++) {

356   uint256 totalRatio = 0;

363   for (uint256 i = 0; i < route.addresses.length; ++i) {

423   uint256 totalTransferred = 0;

424   for (uint256 i = 0; i < addresses.length; ++i) {

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L126

file:  contracts/StakedUSDeV2.sol

83    userCooldown.cooldownEnd = 0;'

84    userCooldown.underlyingAmount = 0;

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/StakedUSDeV2.sol#L83

[G-10] Cache external calls outside of loop to avoid re-calling function on each iteration

Performing STATICCALLs that do not depend on variables incremented in loops should always try to be avoided within the loop. In the following instances, we are able to cache the external calls outside of the loop to save a STATICCALL (100 gas) per loop iteration.

file: contracts/EthenaMinting.sol

424    for (uint256 i = 0; i < addresses.length; ++i) {
      uint256 amountToTransfer = (amount * ratios[i]) / 10_000;
      token.safeTransferFrom(benefactor, addresses[i], amountToTransfer);
      totalTransferred += amountToTransfer;
    }

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L424-L428

[G-11] Use assembly to perform efficient back-to-back calls

If similar external calls are performed back-to-back, we can use assembly to reuse any function signatures and function parameters that stay the same. In addition, we can also reuse the same memory space for each function call (scratch space + free memory pointer), which can potentially allow us to avoid memory expansion costs. In this case, we are also able to efficiently store the function signatures together in memory as one word, saving multiple MLOADs in the process.

Note: In order to do this optimization safely we will cache the free memory pointer value and restore it once we are done with our function calls. We will also set the zero slot back to 0 if neccessary.

file:  main/contracts/StakedUSDeV2.sol

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

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

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/StakedUSDeV2.sol#L100-L101

[G-12] Use calldata instead of memory for function arguments that do not get mutated

When you specify a data location as memory, that value will be copied into memory. When you specify the location as calldata, the value will stay static within calldata. If the value is a large, complex type, using memory may result in extra memory expansion costs.

file: contracts/EthenaMinting.sol

111     constructor(
    IUSDe _usde,
    address[] memory _assets,
    address[] memory _custodians,
    address _admin,
    uint256 _maxMintPerBlock,
    uint256 _maxRedeemPerBlock
  ) {

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L111-L118

[G-13] Use hardcoded address instead of address(this)

Instead of using address(this), it is more gas-efficient to pre-calculate and use the hardcoded address. Foundry’s script.sol and solmate’s LibRlp.sol contracts can help achieve this. Refrences

file:  contracts/EthenaMinting.sol

403    if (address(this).balance < amount) revert InvalidAmount();

452    return keccak256(abi.encode(EIP712_DOMAIN, EIP_712_NAME, EIP712_REVISION, block.chainid, address(this)));

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L403

file:  contracts/StakedUSDe.sol

96    IERC20(asset()).safeTransferFrom(msg.sender, address(this), amount);

167   return IERC20(asset()).balanceOf(address(this)) - getUnvestedAmount();

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

file: contracts/StakedUSDeV2.sol

43   silo = new USDeSilo(address(this), address(_asset));

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/StakedUSDeV2.sol#L43

[G-14] Use uint256(1)/uint256(2) instead for true and false boolean states

Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from ‘false’ to ‘true’, after having been ‘true’ in the past. see source: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27

file: contracts/EthenaMinting.sol

86    mapping(address => mapping(address => bool)) public delegatedSigner;

250   (bool success,) = wallet.call{value: amount}("");

265   function isSupportedAsset(address asset) external view returns (bool) {

339   function verifyOrder(Order calldata order, Signature calldata signature) public view override returns (bool, bytes32) {

377   function verifyNonce(address sender, uint256 nonce) public view override returns (bool, uint256, uint256, uint256) {

391   function _deduplicateOrder(address sender, uint256 nonce) private returns (bool) {

392   (bool valid, uint256 invalidatorSlot, uint256 invalidator, uint256 invalidatorBit) = verifyNonce(sender, nonce);

404   (bool success,) = (beneficiary).call{value: amount}("");

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L86

file: contracts/EthenaMinting.sol

236   delegatedSigner[_delegateTo][msg.sender] = true;

242   delegatedSigner[_removedSigner][msg.sender] = false;

347   return (true, taker_order_hash);

354   return true;

358   return false;

361   return false;

366   return false;

371   return false;

373   return true;

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

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L236

file:  contracts/StakedUSDe.sol

106    function addToBlacklist(address target, bool isFullBlacklisting)

120    function removeFromBlacklist(address target, bool isFullBlacklisting)

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

[G-15] Use assembly to validate msg.sender

We can use assembly to efficiently validate msg.sender for the didPay and uniswapV3SwapCallback functions with the least amount of opcodes necessary. Additionally, we can use xor() instead of iszero(eq()), saving 3 gas. We can also potentially save gas on the unhappy path by using scratch space to store the error selector, potentially avoiding memory expansion costs.

file: contracts/EthenaMinting.sol

138   if (msg.sender != _admin) {

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L138

file: contracts/SingleAdminAccessControl.sol

26    if (newAdmin == msg.sender) revert InvalidAdminChange();

32    if (msg.sender != _pendingDefaultAdmin) revert NotPendingAdmin();

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/SingleAdminAccessControl.sol#L26

file: contracts/USDe.sol

29      if (msg.sender != minter) revert OnlyMinter();

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/USDe.sol#L29

file: contracts/USDeSilo.sol

24   if (msg.sender != STAKING_VAULT) revert OnlyStakingVault();

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/USDeSilo.sol#L24

[G-16] A modifier used only once and not being inherited should be inlined to save gas

file: contracts/EthenaMinting.sol

97    modifier belowMaxMintPerBlock(uint256 mintAmount) {

104   modifier belowMaxRedeemPerBlock(uint256 redeemAmount) {

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L97

[G-17] Use assembly for loops

In the following instances, assembly is used for more gas efficient loops. The only memory slots that are manually used in the loops are scratch space (0x00-0x20), the free memory pointer (0x40), and the zero slot (0x60). This allows us to avoid using the free memory pointer to allocate new memory, which may result in memory expansion costs.

Note that in order to do this optimization safely we will need to cache and restore the free memory pointer after the loop. We will also set the zero slot (0x60) back to 0.

file: contracts/EthenaMinting.sol

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

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

424   for (uint256 i = 0; i < addresses.length; ++i) {
      uint256 amountToTransfer = (amount * ratios[i]) / 10_000;
      token.safeTransferFrom(benefactor, addresses[i], amountToTransfer);
      totalTransferred += amountToTransfer;
    }

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L126-L128

[G-18] Should use arguments instead of state variable

state variables should not used in emit , This will save near 97 gas

file: contracts/EthenaMinting.sol

439   emit MaxMintPerBlockChanged(oldMaxMintPerBlock, maxMintPerBlock);

446   emit MaxRedeemPerBlockChanged(oldMaxRedeemPerBlock, maxRedeemPerBlock);

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L439

file: contracts/StakedUSDeV2.sol

133  emit CooldownDurationUpdated(previousDuration, cooldownDuration);

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/StakedUSDeV2.sol#L133

[G-19] Can make the variable outside the loop to save gas

When you declare a variable inside a loop, Solidity creates a new instance of the variable for each iteration of the loop. This can lead to unnecessary gas costs, especially if the loop is executed frequently or iterates over a large number of elements.

By declaring the variable outside the loop, you can avoid the creation of multiple instances of the variable and reduce the gas cost of your contract. Here’s an example:

contract MyContract {
    function sum(uint256[] memory values) public pure returns (uint256) {
        uint256 total = 0;
        
        for (uint256 i = 0; i < values.length; i++) {
            total += values[i];
        }
        
        return total;
    }
}

Making variable inside the loop gas value: 662604

Making variable outside the loop gas value: 661873

Tools used Remix

Consider making the stack variables before the loop which gonna save gas

file: contracts/EthenaMinting.sol

424   for (uint256 i = 0; i < addresses.length; ++i) {
      uint256 amountToTransfer = (amount * ratios[i]) / 10_000;
      token.safeTransferFrom(benefactor, addresses[i], amountToTransfer);
      totalTransferred += amountToTransfer;
    }

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L424-L428

[G-20] abi.encode() is less efficient than abi.encodepacked()

In terms of efficiency, abi.encodePacked() is generally considered to be more gas-efficient than abi.encode(), because it skips the step of adding function signatures and other metadata to the encoded data. However, this comes at the cost of reduced safety, as abi.encodePacked() does not perform any type checking or padding of data.

file: contracts/EthenaMinting.sol

321  return abi.encode(

335  return abi.encode(ROUTE_TYPE, route.addresses, route.ratios);

452  return keccak256(abi.encode(EIP712_DOMAIN, EIP_712_NAME, EIP712_REVISION, block.chainid, address(this)));

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L321

[G-21] Using delete statement can save gas

file: contracts/EthenaMinting.sol

242  delegatedSigner[_removedSigner][msg.sender] = false;

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L236

[G-22] Use Modifiers Instead of Functions To Save Gas

Example of two contracts with modifiers and internal view function:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.9;
contract Inlined {
    function isNotExpired(bool _true) internal view {
        require(_true == true, "Exchange: EXPIRED");
    }
function foo(bool _test) public returns(uint){
            isNotExpired(_test);
            return 1;
    }
}
// SPDX-License-Identifier: MIT
pragma solidity 0.8.9;
contract Modifier {
modifier isNotExpired(bool _true) {
        require(_true == true, "Exchange: EXPIRED");
        _;
    }
function foo(bool _test) public isNotExpired(_test)returns(uint){
        return 1;
    }
}
file: main/contracts/SingleAdminAccessControl.sol

65    function owner() public view virtual returns (address) {
        return _currentDefaultAdmin;
    }

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/SingleAdminAccessControl.sol#L65-L67

file: main/contracts/SingleAdminAccessControl.sol

184  function decimals() public pure override(ERC4626, ERC20) returns (uint8) {
    return 18;
  }

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/SingleAdminAccessControl.sol#L184-L186

file: main/contracts/EthenaMinting.sol
 
265  function isSupportedAsset(address asset) external view returns (bool) {
    return _supportedAssets.contains(asset);
  }

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L265-L267

[G-23] Gas saving is achieved by removing the delete keyword (~60k)

30k gas savings were made by removing the delete keyword. The reason for using the delete keyword here is to reset the struct values (set to default value) in every operation. However, the struct values do not need to be zero each time the function is run. Therefore, the delete keyword is unnecessary. If it is removed, around 30k gas savings will be achieved.

file: main/contracts/SingleAdminAccessControl.sol

92    delete _pendingDefaultAdmin;

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/SingleAdminAccessControl.sol#L92

[G‑24] Counting down in for statements is more gas efficient

Counting down is more gas efficient than counting up because neither we are making zero variable to non-zero variable and also we will get gas refund in the last transaction when making non-zero to zero variable.

by changing this logic you can save 12171 gas per one for loop

Tools used Remix

file: main/contracts/EthenaMinting.sol

126   for (uint256 i = 0; i < _assets.length; i++) {
   
139   for (uint256 j = 0; j < _custodians.length; j++) {

363   for (uint256 i = 0; i < route.addresses.length; ++i) {

424   for (uint256 i = 0; i < addresses.length; ++i) {

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L126

#0 - c4-pre-sort

2023-11-01T15:11:46Z

raymondfam marked the issue as sufficient quality report

#1 - c4-judge

2023-11-10T20:29:12Z

fatherGoose1 marked the issue as grade-b

Awards

14.2357 USDC - $14.24

Labels

analysis-advanced
grade-b
sufficient quality report
A-11

External Links

Phase 1: Documentation and Video Review

A: Start with a comprehensive review of all documentation related to the Ethena Labs platform, including the whitepaper, API documentation, developer guides, user guides, and any other available resources.

B: Watch any available walkthrough videos to gain a holistic understanding of the system. Pay close attention to any details about the platform’s architecture, operation, and possible edge cases.

C: Note down any potential areas of concern or unclear aspects for further investigation.

Phase 2: Manual Code Inspection

I: Once familiarized with the operation of the platform, start the process of manual code inspection.

II: Review the codebase section by section, starting with the core smart contract logic. Pay particular attention to areas related to EthenaMinting.sol contract ,USDe.sol, StakedUSDe.sol, StakedUSDeV2.sol and USDeSilo.sol contracts

III: Look for common vulnerabilities such as reentrancy, integer overflow/underflow, improper error handling, etc., while also ensuring the code conforms to best programming practices.

IV: Document any concerns or potential issues identified during this stage.

Phase 3: Architecture recommendations:

Ownership Multisig Security: Using a multisig wallet for ownership of smart contracts is a secure approach. However, it's crucial to ensure that the multisig wallet remains highly secure and that access protocols are well-documented and followed strictly. Gatekeeper System: The gatekeeper system to prevent unauthorized minting and redemption is a good security measure. It's recommended to continuously monitor and update the gatekeepers' actions to adapt to changing market conditions and potential risks.

1. USDe.sol: Ownership and Permissions: The code employs the Ownable2Step pattern for ownership, which ensures that administrative actions require multiple steps. This adds an extra layer of security, making it harder for a single entity to execute critical actions. Permit Functionality: The use of the ERC20Permit extension is a valuable feature, allowing users to interact with the token without the need for separate approvals. 2. EthenaMinting.sol: Ownership and Permissions: The code uses the SingleAdminAccessControl pattern for managing roles and permissions, which can enhance security by limiting the scope of who can mint or redeem tokens. EIP-712 and Signatures: The contract uses EIP-712 for structured data hashing and ECDSA signatures to verify the orders, which is a recommended approach for secure transaction signing. 3. StakedUSDe.sol: Access Control: The contract uses OpenZeppelin's AccessControl to manage different roles such as REWARDER_ROLE, BLACKLIST_MANAGER_ROLE, SOFT_RESTRICTED_STAKER_ROLE, and FULL_RESTRICTED_STAKER_ROLE. This is a good practice for managing permissions within the contract. Time-based Vesting: The contract implements a time-based vesting mechanism for rewards. This ensures that rewards are vested over time, preventing instant withdrawal and encouraging long-term staking. This is a good approach to incentivize users to stay engaged with the platform. 4. StakedUSDeV2.sol: Inheritance and Interface: The contract inherits from both StakedUSDe and IStakedUSDeCooldown. This implies that it extends the functionality of StakedUSDe to incorporate cooldown mechanisms for withdrawals and redemptions, which is a reasonable architectural decision. Cooldown Mechanism: The contract introduces a cooldown mechanism that allows users to stake and unstake assets, enabling the staking of shares and the eventual claiming of underlying assets with a cooldown period. This mechanism can be useful in certain DeFi applications, but it adds complexity to the contract, so it's essential to clearly communicate how it works to users. 5. USDeSilo.sol: Single Responsibility: The "USDeSilo" contract has a clear and single responsibility, which is to temporarily hold USDe tokens during the stake cooldown process. This separation of concerns is a good architectural practice. Address Configuration: The addresses for the staking vault and USDe token are passed to the constructor, allowing for flexibility in configuration. Modifier Usage: The contract utilizes a custom modifier onlyStakingVault, ensuring that only the staking vault contract can call the withdraw function. This is a good practice to control access. 6. SingleAdminAccessControl.sol: Single Admin Role: The contract is designed to provide a single admin role. This approach simplifies access control, making it easy to manage and understand. Pending Admin Role: It introduces a concept of a "pending admin" to allow a smooth transition between admin addresses. This is a good practice to prevent abrupt admin role changes. Extending AccessControl: The contract extends OpenZeppelin's AccessControl, which is a well-established and secure library for managing access control in smart contracts.

Phase 4: Codebase quality analysis:

Modularity: The use of multiple smart contracts to separate functions and responsibilities is a good practice for code modularity. Roles and Permissions: The role-based access control system is essential for maintaining control over critical actions in the protocol. EIP712 Signatures: The use of EIP712 signatures for minting and redemption provides an additional layer of security. Max Mint per Block: Enforcing a maximum mint per block is a critical security feature to prevent potential exploits. Cooldown Period: The introduction of a cooldown period for unstaking stUSDe adds a layer of protection against flash loan attacks.

1. USDe.sol: Modularity: The code is well-structured and modular, making it easy to understand and maintain. Permissions: The code effectively distinguishes between the owner, who can set the minter address, and the minter, who can mint new tokens. Permit Integration: The integration of ERC20 permit allows for more user-friendly interactions, enhancing the overall quality of the codebase. 2. EthenaMinting.sol: Modularity: The code is well-structured, making it easy to understand and maintain. Delegated Signers: The ability to delegate signing to EOA addresses is a valuable feature for smart contracts. Custodian Addresses: The code handles custody transfers to multiple custodian addresses with defined ratios, providing flexibility. Supported Assets: The contract allows for dynamic addition and removal of supported assets, enhancing adaptability. Nonce Deduplication: The code enforces the uniqueness of nonces, preventing replay attacks. Security Checks: The code has several checks to ensure the validity of transactions, protecting against potential issues. 3. StakedUSDe.sol: SafeMath: The code does not use SafeMath or a similar library for arithmetic operations. While Solidity version 0.8 automatically checks for overflows and underflows, it's recommended to include SafeMath as an extra precaution, especially for large codebases. Error Handling: The code properly uses revert statements to handle errors. It provides meaningful error messages, making it easier to understand the cause of a failed transaction. Use of External Libraries: The contract leverages external libraries from OpenZeppelin for common functionality. This is a good practice as it promotes code reuse and helps in avoiding common vulnerabilities. Access Control: The contract uses access control roles effectively, ensuring that only authorized users can perform specific actions. This is crucial for the security and integrity of the contract. Blacklisting: The contract allows for the blacklisting of addresses with the SOFT_RESTRICTED_STAKER_ROLE and FULL_RESTRICTED_STAKER_ROLE. This can be useful to restrict certain addresses from interacting with the contract. Minimum Shares: The contract enforces a minimum share requirement to prevent a "donation attack." This ensures that small, potentially malicious transactions are not processed. This is a good security measure. 4. StakedUSDeV2.sol: Safety Measures: The contract inherits from OpenZeppelin's contracts (SafeERC20, IERC20) and implements error handling to ensure safe operations, which is a good practice. Modifiers: The codebase uses modifiers like ensureCooldownOff and ensureCooldownOn to ensure that the cooldown mechanism is enabled or disabled as needed. Constants: It defines the MAX_COOLDOWN_DURATION as a constant. Using constants for important parameters makes it easier to modify them in one place if necessary. Silo Integration: The contract utilizes the USDeSilo contract to manage siloed assets during the cooldown period. This separation of concerns is a good practice to manage assets independently of the core staking functionality. Configuration via Constructor: The contract sets the cooldownDuration during the constructor. This allows for a customizable cooldown duration when deploying the contract. 5. USDeSilo.sol: SafeERC20: The contract uses OpenZeppelin's SafeERC20 library to ensure safe token transfers, which is a best practice to prevent vulnerabilities. Constructor Configuration: Configuration parameters are set in the constructor, ensuring the immutability of these values once the contract is deployed. This prevents accidental changes to critical addresses. 6. SingleAdminAccessControl.sol: Admin Role Management: The contract allows for the transfer of the admin role to a new address and handles this transition through the "pending admin" mechanism. This ensures that changes in the admin role are explicit and avoid sudden transitions. Role Granting and Revocation: The contract provides functions for granting and revoking roles other than the admin role. Proper access control functions are important for managing permissions within a contract. Owner Function: The owner function returns the address of the current admin, which aligns with standard expectations. Modular: The contract is modular, focusing solely on access control, which is a good practice for separation of concerns.

Phase 5: Centralization risks:

Gatekeepers: The role of gatekeepers is pivotal in maintaining protocol integrity. The potential involvement of external organizations as gatekeepers should be carefully considered to avoid undue centralization.

1. USDe.sol: Owner's Role: The owner has the power to set the minter address, which can be a potential centralization point. It's essential to ensure that the owner address is secure and not subject to unauthorized changes. 2. EthenaMinting.sol: Role Management: The roles of "MINTER_ROLE," "REDEEMER_ROLE," and "GATEKEEPER_ROLE" should be carefully managed, as they control critical functionality. It's crucial to ensure that the gatekeeper role remains secure to prevent unauthorized changes to minting and redeeming. 3. StakedUSDe.sol: The contract have certain roles that are controlled by administrators (e.g., REWARDER_ROLE, BLACKLIST_MANAGER_ROLE, DEFAULT_ADMIN_ROLE). If not implemented and managed carefully, centralization of these roles can be a risk to decentralization and security. It is essential to ensure that control is distributed among multiple, trusted parties. 4. StakedUSDeV2.sol: Role Management: The contract inherits the role management mechanism from StakedUSDe. Centralization risks associated with the control roles remain similar to the base contract. Proper management and distribution of these roles are essential for decentralization and security.\ 5. USDeSilo.sol: Access Restriction: The contract ensures that only the staking vault contract can call the withdraw function. This restricts access to a specific contract, which can be a central point of control. Proper access control management is important to mitigate centralization risks. 6. SingleAdminAccessControl.sol: Single Admin Role: By design, this contract implements a single admin role. This centralizes control over the contract. The centralization risk is mitigated by the "pending admin" mechanism, which prevents an immediate change of admin address.

Phase 6: Mechanism Review:

Minting and Redeeming: The minting and redeeming mechanisms appear to be well-defined and include safeguards such as EIP712 signatures, role-based access control, and block limits. Staking and Yield Generation: The staking mechanism is designed to provide users with stUSDe and yield. It's crucial to ensure that the yield generation is efficient and secure.

1. USDe.sol: Minting Function: The mint function allows the designated minter to create new USDe tokens, and the onlyMinter modifier restricts this ability to the approved minter address. The mechanism appears sound and secure. 2. EthenaMinting.sol: Minting and Redeeming: The minting and redeeming mechanisms appear sound and secure, with proper checks and balances, including nonce-based deduplication. 3. StakedUSDe.sol: Staking Mechanism: The contract provides a staking mechanism for users to stake their USDe tokens and receive stUSDe tokens in return. The stUSDe tokens represent a share of protocol rewards. Vesting: The contract incorporates a vesting mechanism for rewards. This means that rewards are not immediately available for withdrawal, encouraging long-term participation. Blacklisting: The contract allows for blacklisting addresses, which could be useful to prevent malicious users from participating in the system. 4. StakedUSDeV2.sol: Withdrawal and Redemption: The contract disables the standard withdrawal and redemption functions from ERC4626 when the cooldown mechanism is active. Instead, users need to use cooldownShares and cooldownAssets for interacting with their assets. This change breaks the ERC4626 standard, so it's important to communicate this clearly to users. Unstaking and Claiming: The contract introduces the concept of unstaking and claiming after the cooldown period. This offers an alternative way for users to access their assets, which is valuable in certain use cases. Cooldown Duration: The contract allows the administrator to set the cooldown duration. This flexibility is important to adapt to different project requirements. 5. USDeSilo.sol: Withdraw Function: The withdraw function allows the staking vault to withdraw USDe tokens from the silo. This function is protected by the onlyStakingVault modifier, which ensures that only authorized contracts can perform withdrawals. 6. SingleAdminAccessControl.sol: Pending Admin: The "pending admin" mechanism ensures that changes in the admin role are subject to a transition period, which adds a layer of security and protection against abrupt changes in control.

Phase 7: Systemic Risks:

1. USDe.sol: Minter Security: The security of the minter address is critical. If the minter address is compromised, it could lead to unauthorized minting of tokens. Therefore, it's crucial to protect the minter's private key securely. 2. EthenaMinting.sol: Replay Attacks: The code uses nonce-based deduplication to prevent replay attacks. However, the security of the nonce generation and management is crucial to avoid potential issues. 3. StakedUSDe.sol: Restricted Roles: The introduction of roles like FULL_RESTRICTED_STAKER_ROLE should be used with caution. The contract owner has the ability to redistribute locked amounts, but this should be carefully managed to prevent any potential misuse. Vesting Duration: The vesting period is set to 8 hours. Depending on the specific use case, this duration may need to be adjusted to align with the desired behavior of the protocol. Centralization: As mentioned earlier, the contract relies on certain roles, which could lead to centralization if not managed properly. A multisig or decentralized control mechanism could be considered for enhanced decentralization. Access Control: The access control mechanism should be rigorously audited to ensure that only trusted parties are granted sensitive roles. 4. StakedUSDeV2.sol: Change in Standard: The contract changes the behavior of the underlying ERC4626 standard, which might affect how other applications interact with the tokens. This needs to be communicated clearly to potential users. Coordinated Actions: The contract allows users to stake, unstake, and claim their assets, which could potentially lead to coordinated actions that impact the project's operation. Careful design and risk assessment are required to mitigate this. 5. USDeSilo.sol: Control Over USDe Tokens: The "USDeSilo" contract holds USDe tokens during the cooldown process. It's important to consider the control over these tokens and potential implications for the broader system. Access to the silo should be carefully managed to avoid any centralization risks or unintended consequences. Interoperability: This contract is designed to work in conjunction with other contracts in the system, specifically with the staking vault. Proper coordination and integration between contracts are essential to ensure the overall system's functionality. 6. SingleAdminAccessControl.sol: Admin Key Security: The security of the admin address is crucial since it has significant control over the contract. Proper key management and security practices for the admin key are essential to prevent unauthorized access. Admin Role Assignment: The contract should ensure that the initial assignment of the admin role is secure and that the admin key is properly protected.

Phase 8: Recommendations:

Admin Key Security: Ensure that the admin key, which has significant control over the contract, is stored securely and follows best practices for key management. Use hardware wallets or other secure storage solutions to protect the admin key.

Access Control Best Practices: Continue following best practices for access control. Ensure that only authorized addresses have the admin role and that access control functions are correctly implemented.

User Documentation: Provide clear and comprehensive user documentation to guide users on how to interact with the protocol, including instructions on staking, unstaking, and any other relevant actions.

Systemic Risk Assessment: Conduct a systemic risk assessment to identify and mitigate any potential risks or vulnerabilities that might arise from the interaction of various components in the protocol.

Governance Model: Implement a transparent governance model that allows users to have a say in the decision-making process, such as changes to parameters, upgrades, or changes to the admin key.

External Security Review: Seek external reviews and feedback from the developer community and security experts. Consider running bug bounty programs to encourage external auditors to review the code.

Phase 9: Time Spent

25 hours

Time spent:

25 hours

#0 - c4-pre-sort

2023-11-01T14:23:21Z

raymondfam marked the issue as sufficient quality report

#1 - c4-judge

2023-11-10T19:36:41Z

fatherGoose1 marked the issue as grade-b

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