Ethena Labs - Bulletprime's results

Description overview of Ethena Ethena, a groundbreaking synthetic dollar USDe protocol built on the Ethereum blockchain, aims to revolutionize the concept of money by offering a crypto-native alternative that doesn't rely on traditional banking infrastructure. This innovative platform also introduces a globally accessible savings instrument called the 'Internet Bond', providing users with a dollar-denominated investment opportunity. With Ethena, individuals can enjoy the benefits of a decentralized financial system while retaining the stability and accessibility of a traditional savings instrument. The system’s primary purpose is to offer a Stable coin solution powered by ETH that achieves price stability while providing staking returns to users. By leveraging futures arbitrage across centralized and decentralized platforms, the protocol aims to maintain stable coin's value. Ethena offers users the ability to mint/redeem a delta-neutral stable coin, USDe, that returns a sustainable yield to the holder. The yield is generated through staking Ethereum and the funding rate earned from the perpetual position hedging the delta. System overview The primary functions used in this contract is the mint() and redeem(). n users who are able to call this contract are all within Ethena. When outside users wishes to mint or redeem, they perform an EIP712 signature based on an offchain prices that are provided to them. They sign the order and sends it back to ethenas end, where it is run through series of checks and are the ones who take their signed order and put them on chain. By construct, Ethena will be the only ones along the mint(), redeem(). Important contracts StakedUSDeV2.sol Contract where holders of USDe stablecoin can stake their stablecoin, get stUSDe in return and earn yield. protocol's yield is paid out by having a REWARDER role of the staking contract send yield in USDe, increasing the stUSDe value with respect to USDe. USDe.sol The contract for the stablecoin. It extends ERC20Burnable, ERC20Permit and Ownable2Step from Open Zepplin. There's a single variable, the minter address that can be modified by the OWNER. EthenaMinting.sol The contract and address that the minter variable in USDe.sol points to. When users mint USDe with stETH (or other collateral) or redeems collateral for USDe, this contract is invoked

Code base approach 2.1 Audit Documentation and Scope Initial step involved, was thoroughly examining audit documentation and scope to grasp the audit’s functionalities and boundaries, and priorities my efforts. It is worth highlighting the good quality of the READMEfor this audit, as it provided valuable insights and actionable guidance that greatly facilitated the onboarding process.

2.2 Inheritance A good amount of hour was put in analyzing the inheritance used, as the contract code used a lot of inheritance from open zepplein, it was important to review imported contracts and interfaces for proper implementation, tested and reviewed for potential vulnerabilities.

2.3 Setup and Testing Setting up test environment to execute forge test was remarkably effortless, greatly enhancing the efficiency of the auditing process. With a fully functional test tap at our disposal, which not only accelerate the testing of intricate concepts and potential vulnerabilities but also gain insights into the developer’s expectations regarding the implementation.

2.4 Code review The code review commenced with understanding the inheritance pattern used to manage authorization and functionality across the system. Thoroughly understanding this pattern made understanding the protocol contracts and its relations much smoother. Throughout this stage, I documented observations and raised questions concerning potential exploits without going too deep.

2.5 Threat Modelling At this point I began formulating specific assumptions that, if compromised, could pose security risks to the system. This process aids me in determining the most effective exploitation strategies. Thoroughly examining and marking any doubtful or vulnerable areas within the protocol, diving deep into these areas, performing in-depth examinations, and subjecting them to rigorous testing to report these issues if they don't checkout.

2.6 Known Findings Read old audits and already known findings. Went through the bot races findings checking what was found so as to not report these issues again, while bearing in mind the kind of bugs the contract is concerned about.

2.7 Report Issues I started with auditing the code base indepthly this way I started understanding line by line code and took the necessary notes to ask questions, marking out vulnerabilities and how they can be exploited, This assessment helps in evaluating whether these exploits can be strategically combined to create a more significant impact on the system’s security. In some cases, seemingly minor and moderate issues can compound to form a critical vulnerability when leveraged wisely. This has to be balanced with any risks that users may face.

Architecture overview The platform supports a generic implementation and lots of inheritance, and as the team understands, this leads to a broad variety of malicious deployed risks. Although i must commend the considered various security measures and roles to mitigate potential risks. To highlight a few, Minter and Redeemer Roles: It's good to have separate roles for minting and redeeming USDe. By limiting the minting and redeeming capability to specific roles, you can control the creation and redemption of USDe tokens.

Restricted Staker Roles: The inclusion of soft and full restricted staker roles is a good approach to comply with legal requirements and prevent misuse of the protocol. However, it's important to have a robust process in place to verify and enforce these restrictions. Clear guidelines and procedures should be established to handle restricted stakers and address any potential legal or compliance issues.

Vesting and Cooldown Periods: The implementation of a vesting period for rewards and a cooldown period for unstacking stUSDe adds an additional layer of security and prevents front-running. It's important to ensure that these periods are properly implemented and enforced to maintain the fairness and integrity of the protocol.

Silo Contract: The use of a separate silo contract to hold funds during the cooldown period is a good practice to ensure the availability of funds for withdrawal. Overall, the architecture seems to have considered various security measures and roles to mitigate potential risks that i must commend. However, it's crucial to regularly review and update these measures, conduct security audits, and stay updated with the latest best practices in the industry to ensure the ongoing security of the protocol.

Code commentary In as much as the documentation of this protocol provides a very streamlined understanding of its functionality, the code base approach still needs to be totally comprehensive to strike the balance. My suggestions to areas of improvements, Codebase could totally benefit from consistent and latest solidity version plus the bug fixes benefits that comes with it, also use the latest version of openzepplein.

Generally a best practice to make lines in source code to be within certain length, 80-120 for comprehensive readability, there are several instance where Multiline output parameters and return statements did not follow the same style recommended for wrapping long lines found in the Maximum Line Length section. Instance

```solidity```
File: contracts/StakedUSDe.sol
/// @notice The role which prevents an address to transfer, stake, or unstack. The owner of the contract can redirect address staking balance if an address is in full restricting mode.

Other lapses found from going through the code base was where the solidity layout ordering guide where misplaced, which can distort readability. its best to follow solidity best guide for a more streamlined code base.

This protocol made use of a lot of imports there were Presence of Unutilized Imports in the Contract, The contract contains import statements for libraries and other contracts that are not utilized within the code. Excessive or unused imports can clutter the codebase, leading to inefficiency and potential confusion. Consider removing any imports that are not essential to the contract's functionality. State or Local Variable names in your contract don't align with the Solidity naming convention. For clarity and code consistency, it's recommended to use mixed Case for local and state variables that are not constants, as it helps improve readability.

Due to how complex this code base is and its amount of inheritance it should include invariant fuzzing tests, These tests require the identification of invariants that should not be violated under any circumstances, with the fuzzer testing various inputs and function calls to ensure these invariants always hold. This is especially important for code with a lot of inline-assembly, or complex interactions between contracts. Despite having up to 80% code coverage, bugs can still occur due to the order of operations a user performs. My recommendation is to add these tests to verify behavior of the system interaction as a whole.

Using a boolean parameter in the addtoblacklist function stakedUSDe.sol contract, to determine whether to perform a full or soft blacklisting is a valid approach for this function. It allows the caller to specify the type of blacklisting they want to apply. However, it's important to note that using a boolean parameter can make the code less readable and harder to understand. It may be more clear and maintainable to use an enum type instead of a boolean. The enum could have two values, such as BlacklistType.FULL and BlacklistType.SOFT, which would make the intent of the parameter more explicit.

```solidity```
enum BlacklistType {
  FULL,
  SOFT
}

function addToBlacklist(address target, BlacklistType blacklistType)
    external
    onlyRole(BLACKLIST_MANAGER_ROLE)
    notOwner(target)
{
    bytes32 role = blacklistType == BlacklistType.FULL ? FULL_RESTRICTED_STAKER_ROLE : SOFT_RESTRICTED_STAKER_ROLE;
    _grantRole(role, target);
}

Centralization risks A single point of failure is not acceptable for this project Centrality risk potentially arises, as it uses a single ownership transfer in constructor, A single-step ownership transfer in the constructor can indeed pose a centralization risk. This is because the ownership of the contract is transferred to a specific address during contract deployment, often the address of the deployer. If this address is compromised, it can lead to a complete loss of control over the contract. To help mitigate this risk, it's recommended to follow the principle of least privilege. The contract should not grant unnecessary permissions to any address, especially during contract deployment. Instead, the contract should implement a multi-signature or multi-factor authentication mechanism to transfer ownership, requiring the approval of multiple trusted parties. Additionally, the contract should include a mechanism to revoke or change the role of an address in case it is compromised. This can be achieved by implementing a role registry that tracks the roles of all addresses and can be updated by the contract owner. Remember, security should be a top priority in smart contract development.

Mechanism Review A mechanism review would involve a thorough analysis of the contract's functionality, its use of security mechanisms like access control and reentrancy protection, and its potential vulnerabilities. Potential areas for mechanism review might include:

Access Control: The contract uses role-based access control, which is a good security practice. However, the contract should also include a mechanism to revoke or change the role of an address in case it is compromised. This can be achieved by implementing a role registry that tracks the roles of all addresses and can be updated by the contract owner.

Reentrancy Protection: The contract includes a nonReentrant modifier to prevent reentrancy attacks. This is a good security measure. However, the nonReentrant modifier should be applied to all functions that perform external calls, not just the redeem and mint functions.

Gas Limit Considerations: The mint() and redeem() functions should be designed to avoid exceeding the block gas limit, especially if they involve complex operations or external calls.

Error Handling: The contract uses revert statements to handle errors and revert transactions when necessary. However, the error messages could be more informative to help users understand why a transaction failed.

Code Organization: The code could benefit from better organization and more comments to explain the purpose and functionality of each function.

Possible System Risk Third-Party Dependency Risk: Contracts rely on external data source, such as @openzeppelin/contracts, and there is a risk that if any issues are found within these dependencies in your contracts, protocols could definitely also be affected. I also observed that old versions of OpenZeppelin are used in the project, and these should be updated to the latest version.

Update Risk: If the smart contract needs to be updated, there is a risk that updates may be implemented incorrectly or that new versions introduce vulnerabilities.

Smart Contract Vulnerability Risk: Smart contracts can contain vulnerabilities that can be exploited by attackers, cause of the use of inheritance. If a smart contract has critical security flaws, such as access errors or logic problems, this could lead to asset loss or system manipulation. We strongly recommend that, once the protocol is audited, necessary actions be taken to mitigate any issues identified by C4 Wardens.

Test Coverage Risks: With only 80% test coverage, the protocol may have undiscovered vulnerabilities. Inadequate security audits could leave critical issues unnoticed, increasing the risk of attacks. The team should prepare multiple recovery scenarios and set up adequate action channels to prepare for eventualities.

Possible Gas optimization

1. Removing the unnecessary onlyOwner modifier from the mint functions. Since the contract already inherits from Ownable, the onlyOwner modifier is redundant in this case.
2. Gas optimization can also be achieved by removing the virtual keyword from the mint functions if there is no intention to override them in derived contracts.

Conclusion Auditing this codebase and its architectural choices has been a rewarding experience. The project has managed to strike a delicate balance between simplicity and complexity, making it a robust and efficient system. I have provided a comprehensive review of the methodology used during the audit of the contracts within scope, along with valuable insights for the project team and other stakeholders. The codebase demonstrates a understanding of smart contract security, making it a reliable foundation for the project's goals. Remember, security is a continuous process and these are just initial observations. A thorough security review and testing are recommended to identify and address any potential vulnerabilities. Ultimately, the continued success of Ethena will depend on its ability to address these challenges, especially after potential security issues are disclosed by C4 wardens

Time Spent

A total of 3 days was dedicated to completing this audit, distributed as follows:

1st Day: Devoted to comprehending the protocol’s functionalities and implementation. 2nd Day: Initiated the analysis process, leveraging the documentation offered by the Project 3rd Day: Focused on conducting a thorough analysis, incorporating meticulously crafted mental image derived from the contracts and information provided by the protocol.

Time spent:

19 hours