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
Rank: 69/147
Findings: 3
Award: $25.22
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: 0xmystery
Also found by: 0x11singh99, 0xAadi, 0xAlix2, 0xG0P1, 0xStalin, 0xWaitress, 0x_Scar, 0xhacksmithh, 0xhunter, 0xpiken, Al-Qa-qa, Arz, Avci, Bauchibred, BeliSesir, Breeje, Bughunter101, DarkTower, Eeyore, Fitro, HChang26, Imlazy0ne, J4X, JCK, Kaysoft, Kral01, Madalad, Mike_Bello90, Noro, PASCAL, PENGUN, Proxy, Rickard, Shubham, SovaSlava, Strausses, Team_Rocket, ThreeSigma, Topmark, Udsen, Walter, Yanchuan, Zach_166, ZanyBonzy, adam-idarrha, adeolu, almurhasan, arjun16, ast3ros, asui, ayden, btk, cartlex_, castle_chain, cccz, chainsnake, codynhat, critical-or-high, cryptonue, csanuragjain, deepkin, degensec, dirk_y, erebus, foxb868, ge6a, hunter_w3b, jasonxiale, kkkmmmsk, lanrebayode77, lsaudit, marchev, matrix_0wl, max10afternoon, nuthan2x, oakcobalt, oxchsyston, pavankv, peanuts, pep7siup, pipidu83, pontifex, ptsanev, qpzm, radev_sw, rokinot, rotcivegaf, rvierdiiev, sorrynotsorry, squeaky_cactus, supersizer0x, tnquanghuy0512, twcctop, twicek, young, zhaojie, ziyou-
4.5226 USDC - $4.52
Number | Issue | Instances |
---|---|---|
[L-01] | For loops in public or external functions should be avoided due to high gas costs and possible DOS | 3 |
[L-02] | Use descriptive constant instead of 0 as a parameter | 2 |
[L-03] | Revert on transfer to the zero address | 4 |
[L-04] | Functions calling contracts with transfer hooks are missing reentrancy guards | 3 |
[L-05] | Lack of User Validation in the EthenaMinting.sol | 2 |
[L-06] | Lack of Limitations in the removeSupportedAsset Function | 1 |
[L-07] | When removing _supportedAssets _supportedAssets.remove(asset) should be set to 0 for inactive | 1 |
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
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
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
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
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
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.
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
🌟 Selected for report: 0xVolcano
Also found by: 0x11singh99, 0xAadi, 0xAnah, 0xgrbr, 0xhacksmithh, 0xhex, 0xpiken, 0xta, J4X, JCK, K42, Raihan, Rolezn, SAQ, SM3_SS, Sathish9098, SovaSlava, ThreeSigma, Udsen, arjun16, aslanbek, brakelessak, castle_chain, evmboi32, hunter_w3b, lsaudit, naman1778, niser93, nuthan2x, oakcobalt, pavankv, petrichor, phenom80, radev_sw, shamsulhaq123, tabriz, thekmj, unique, yashgoel72, ybansal2403
6.4563 USDC - $6.46
Number | Issue | Instances | Total gas saved |
---|---|---|---|
[G-01] | Possible Optimization EthenaMinting.sol | 2 | |
[G-02] | Possible Optimization in EthenaMinting.sol | 2 | |
[G-03] | Possible Optimization in verifyNonce() function | 1 | |
[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 parameters | 1 | |
[G-06] | public functions not called by the contract should be declared external instead | 8 | |
[G-07] | Amounts should be checked for 0 before calling a transfer | 3 | |
[G-08] | x += y costs more gas than x = x + y for state variables | 6 | 18 |
[G-09] | Do not initialize variables with default values | 10 | 30 |
[G-10] | Cache external calls outside of loop to avoid re-calling function on each iteration | 1 | |
[G-11] | Use assembly to perform efficient back-to-back calls | 2 | |
[G-12] | Use calldata instead of memory for function arguments that do not get mutated | 1 | |
[G-13] | Use hardcoded address instead of address(this) | 5 | |
[G-14] | Use uint256(1)/uint256(2) instead for true and false boolean states | 20 | 200000 |
[G-15] | Use assembly to validate msg.sender | 5 | 15 |
[G-16] | A modifier used only once and not being inherited should be inlined to save gas | 2 | |
[G-17] | Use assembly for loops | 3 | |
[G-18] | Should use arguments instead of state variable | 3 | |
[G-19] | Can make the variable outside the loop to save gas | 1 | 731 |
[G-20] | abi.encode() is less efficient than abi.encodepacked() | 3 | 153951 |
[G-21] | Using delete statement can save gas | 1 | |
[G-22] | Use Modifiers Instead of Functions To Save Gas | 3 | |
[G-23] | Gas saving is achieved by removing the delete keyword (~60k) | 1 | |
[G-24] | Counting down in for statements is more gas efficient | 4 | 50868 |
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)
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)
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
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 ); }
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
[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)
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
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)
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)
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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; } }
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
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
file: contracts/EthenaMinting.sol 242 delegatedSigner[_removedSigner][msg.sender] = false;
https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/EthenaMinting.sol#L236
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; }
file: main/contracts/SingleAdminAccessControl.sol 184 function decimals() public pure override(ERC4626, ERC20) returns (uint8) { return 18; }
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
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
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
🌟 Selected for report: radev_sw
Also found by: 0xSmartContract, 0xweb3boy, Al-Qa-qa, Bauchibred, Bulletprime, D_Auditor, J4X, JCK, K42, Kral01, Sathish9098, ZanyBonzy, albahaca, catellatech, clara, digitizeworx, fouzantanveer, hunter_w3b, invitedtea, jauvany, oakcobalt, pavankv, peanuts, xiao
14.2357 USDC - $14.24
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.
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.
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.
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.
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.
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.
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.
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.
25 hours
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