Audit name:
[SCA] Covalent / EWM-Lc-Contracts / Oct2024
Date:
Nov 15, 2024
Table of Content
Introduction
We express our gratitude to the Covalent team for the collaborative engagement that enabled the execution of this Smart Contract Security Assessment.
Covalent is a modular data infrastructure layer that’s dedicated to solving the Long-Term Data Availability and the verifiability problem in AI.
Document | |
|---|---|
| Name | Smart Contract Code Review and Security Analysis Report for Covalent |
| Audited By | Niccolò Pozzolini, Viktor Lavrenenko |
| Approved By | Przemyslaw Swiatowiec |
| Website | https://www.covalenthq.com/→ |
| Changelog | 01/11/2024 - Preliminary Report |
| 13/11/2024 - Final Report | |
| Platform | Base |
| Language | Solidity |
| Tags | ERC721, MerkleProof, Whitelist, ERC20 Upgradeable, ERC721Upgradeable, ERC4907 |
| Methodology | https://hackenio.cc/sc_methodology→ |
Document
- Name
- Smart Contract Code Review and Security Analysis Report for Covalent
- Audited By
- Niccolò Pozzolini, Viktor Lavrenenko
- Approved By
- Przemyslaw Swiatowiec
- Website
- https://www.covalenthq.com/→
- Changelog
- 01/11/2024 - Preliminary Report
- 13/11/2024 - Final Report
- Platform
- Base
- Language
- Solidity
- Tags
- ERC721, MerkleProof, Whitelist, ERC20 Upgradeable, ERC721Upgradeable, ERC4907
- Methodology
- https://hackenio.cc/sc_methodology→
Review Scope | |
|---|---|
| Repository | https://github.com/covalenthq/ewm-lc-contracts→ |
| Commit | c94d525 |
Review Scope
- Commit
- c94d525
Audit Summary
The system users should acknowledge all the risks summed up in the risks section of the report
Documentation quality
Functional requirements are complete.
Technical description is partially provided:
Authorization roles are not described.
Architectural overview is missing.
Utilized technologies are not described.
Code quality
The development environment is configured.
Best practices are followed.
Test coverage
Code coverage of the project is 85.06% (branch coverage).
Deployment and basic user interactions are covered with tests.
Negative cases are covered.
System Overview
Covalent EWM is a protocol with the following contracts:
EwmNftAllowance - manages the staking of ERC20 tokens to allow users to mint NFTs, with features like whitelisting, allocation periods, and stake release after an NFT expiry time.
EwmNftClaim - manages the claiming process for NFTs, allowing users to claim their unclaimed NFTs while incorporating features like pausing, blacklisting, and administrative controls.
EwmNftController - ERC721 token contract with features like user delegation and reward distribution.
Privileged roles
EwmNftAllowace owner capabilities:
Set Whitelist Root Hash: The owner can update the whitelistRootHash using the setWhitelistRootHash function.
Set Integer Allocation: The owner can toggle the isIntegerAllowance flag with the setIsIntegerAllocation function.
Pause and Unpause: The owner can pause and unpause the contract using the pause and unpause functions.
Set NFT Controller Address: The owner can update the nftController address with the setNftControllerAddress function.
Set NFT Expiry Time: The owner can update the nftExpiryTime using the setNftExpiryTime function.
Update Allocation Period: The owner can change the startTime and endTime of the allocation period with the updateAllocationPeriod function.
Update Max Total Stakeable: The owner can set a new maxTotalStakeable limit using the updateMaxTotalStakeable function.
EwmNftClaim owner capabilities:
Update Claim Admin: The owner can update the claim admin address using the updateClaimAdmin function.
Pause and Unpause: The owner can pause and unpause the contract using the pause and unpause functions.
EwmNftClaim owner capabilities:
Update Allowance Contracts Array: The claimAdmin can update the _allowanceContractsArray using the updateAllowanceContractsArray function.
Claim NFTs on Behalf of Users: The claimAdmin can claim NFTs for a specific user using the adminClaim function, which bypasses the blacklist.
Batch Claim NFTs: The claimAdmin can perform batch claims for multiple users using the adminBatchClaim function.
Batch Claim All NFTs: The claimAdmin can claim all unclaimed NFTs for multiple users using the adminBatchClaimAll function.
Update Blacklist: The claimAdmin can update the blacklist status of a user using the updateBlacklist function.
EwmNftController owner capabilities:
Initialize the Contract: The owner can initialize the contract with specific addresses for various roles and settings.
Set Base URL: The owner can set the metadata base URL for the NFTs using setBaseUrl.
Set and Update Expiry Ranges: The owner can define and update expiry ranges for token IDs using setExpiryRange and updateExpiryRange.
Update Admin Addresses: The owner can update the addresses for the minter, whitelist, ban, and reward admins using updateMinterAdmin, updateWhitelistAdmin, updateBanAdmin, and updateRewardAdmin.
Update NFT Transferability: The owner can toggle whether NFTs are transferable after expiry using updateNftTransferable.
Manage Rewards: The owner can deposit and withdraw reward tokens from the reward pool using depositRewardTokens and takeOutRewardTokens.
Pause and Unpause: The owner can pause and unpause the contract using pause and unpause.
EwmNftController rewardManager capabilities:
Reward Token Distribution: The rewardManager can call the rewardTokenIds function to distribute rewards to specific token IDs.
EwmNftController whitelistAdmin capabilities:
Update Transfer Whitelist: The whitelistAdminAddress can update the transfer whitelist by calling the updateTransferWhiteList function. This allows adding or removing addresses from the whitelist.
Update Whitelist Transfer Time: The whitelistAdminAddress can set the start and end times for whitelist transfers using the updateWhitelistTransferTime function.
EwmNftController banAdmin capabilities:
Ban/Unban Tokens: The ban function allows the banAdmin to ban a specific token by setting an end time for the ban. This prevents the token from being transferred or used. The unBan function is used to lift the ban.
Potential Risks
If the deployment and the initialization of the implementation contract, which is EwmNftController.sol, will not be handled within one transaction, it can cause the frontrunning of the EwmNftController::initialize() function by the attacker which will lead to unexpected system behavior.
The digital contracts architecture relies on administrative keys for critical operations. Centralized control over these keys presents a significant security risk, as compromise or misuse can lead to unauthorized actions.
In EwmNftAllowance contract, the variable nftExpiryTime is arbitrarily controlled by the contract owner, who can thus decide when to allow the ctx unstake.
The implemented logic highly depends on external contracts not covered by the audit, in particular the payment IERC20Upgradeable token. This reliance introduces risks if this external contract is compromised or contains vulnerabilities, affecting the audited project's integrity.
The system in the scope interacts with the upgradeable ERC20, which is used for payments. However, the previously mentioned contract is not in the scope of the audit. Some ERC20 tokens do not fully comply with the ERC20 interface standards. As a consequence, it can lead to significant issues within the system, such as transaction failures, incompatibility with wallets and exchanges, and increased security vulnerabilities.
Findings
Code ― | Title | Status | Severity | |
|---|---|---|---|---|
| F-2024-6902 | Inability to Redeem Rewards for Expired Tokens Leading to Locked Funds | fixed | High | |
| F-2024-6886 | Denial of Service if expiryRanges Unset | fixed | Low | |
| F-2024-6887 | Missing Validation of Expiry Ranges to Prevent Overlapping | fixed | Low | |
| F-2024-6848 | Use of mint() Instead of safeMint() May Lead to Permanent Loss of Tokens | fixed | Observation | |
| F-2024-6914 | Lack of Input Validation | fixed | Observation | |
| F-2024-6904 | Redundant Variable validTokenIds in redeemRewards Function | fixed | Observation | |
| F-2024-6893 | Redundant Authorization Check in the ban Function | fixed | Observation | |
| F-2024-6890 | Redundant whenNotPaused Modifier in batchSetUser Function | fixed | Observation | |
| F-2024-6889 | Redundant Handling of info.redeemable in _setUser Function | fixed | Observation | |
| F-2024-6888 | Expired Token Delegation in setUser Function Does Not Revert | fixed | Observation |
Identify vulnerabilities in your smart contracts.
Appendix 1. Definitions
Severities
When auditing smart contracts, Hacken is using a risk-based approach that considers Likelihood, Impact, Exploitability and Complexity metrics to evaluate findings and score severities.
Reference on how risk scoring is done is available through the repository in our Github organization:
Severity | Description |
|---|---|
Critical | Critical vulnerabilities are usually straightforward to exploit and can lead to the loss of user funds or contract state manipulation. |
High | High vulnerabilities are usually harder to exploit, requiring specific conditions, or have a more limited scope, but can still lead to the loss of user funds or contract state manipulation. |
Medium | Medium vulnerabilities are usually limited to state manipulations and, in most cases, cannot lead to asset loss. Contradictions and requirements violations. Major deviations from best practices are also in this category. |
Low | Major deviations from best practices or major Gas inefficiency. These issues will not have a significant impact on code execution. |
Severity
- Critical
Description
- Critical vulnerabilities are usually straightforward to exploit and can lead to the loss of user funds or contract state manipulation.
Severity
- High
Description
- High vulnerabilities are usually harder to exploit, requiring specific conditions, or have a more limited scope, but can still lead to the loss of user funds or contract state manipulation.
Severity
- Medium
Description
- Medium vulnerabilities are usually limited to state manipulations and, in most cases, cannot lead to asset loss. Contradictions and requirements violations. Major deviations from best practices are also in this category.
Severity
- Low
Description
- Major deviations from best practices or major Gas inefficiency. These issues will not have a significant impact on code execution.
Potential Risks
The "Potential Risks" section identifies issues that are not direct security vulnerabilities but could still affect the project’s performance, reliability, or user trust. These risks arise from design choices, architectural decisions, or operational practices that, while not immediately exploitable, may lead to problems under certain conditions. Additionally, potential risks can impact the quality of the audit itself, as they may involve external factors or components beyond the scope of the audit, leading to incomplete assessments or oversight of key areas. This section aims to provide a broader perspective on factors that could affect the project's long-term security, functionality, and the comprehensiveness of the audit findings.
Appendix 2. Scope
The scope of the project includes the following smart contracts from the provided repository:
Scope Details | |
|---|---|
| Repository | https://github.com/covalenthq/ewm-lc-contracts→ |
| Commit | c94d52563f7e47f7b0c8e2b13beb43a7a50806dc |
| Whitepaper | https://www.covalenthq.com/docs/resources/ewm-light-client-whitepaper→ |
| Requirements | https://www.covalenthq.com/docs/nodes/ewm-light-client/ewm-lc-home→ |
| Technical Requirements | https://www.covalenthq.com/docs/nodes/ewm-light-client/ewm-lc-home→ |
Scope Details
- Commit
- c94d52563f7e47f7b0c8e2b13beb43a7a50806dc
- Technical Requirements
- https://www.covalenthq.com/docs/nodes/ewm-light-client/ewm-lc-home→
Assets in Scope
Appendix 3. Additional Valuables
Verification of System Invariants
During the audit of **Covalent/EWM-Lc-Contracts]**, Hacken followed its methodology by performing fuzz-testing on the project's main functions. [Echidna →, a tool used for fuzz-testing, was employed to check how the protocol behaves under various inputs. Due to the complex and dynamic interactions within the protocol, unexpected edge cases might arise. Therefore, it was important to use fuzz-testing to ensure that several system invariants hold true in all situations.
Fuzz-testing allows the input of many random data points into the system, helping to identify issues that regular testing might miss. A specific Echidna fuzzing suite was prepared for this task, and throughout the assessment, 7 invariants were tested over 5,000,000 runs. This thorough testing ensured that the system works correctly even with unexpected or unusual inputs.
Invariant | Test Result | Run Count |
|---|---|---|
EwmNftAllowance::setNftExpiryTime() should always revert for the incorrect _nftExpiryTime | Passed | 5M+ |
EwmNftAllowance::setNftExpiryTime() does not revert for the valid _nftExpiryTime | Passed | 5M+ |
EwmNftAllowance::updateAllocationPeriod() should always revert for the incorrect allocation period: startTime and endTime | Passed | 5M+ |
EwmNftAllowance::updateAllocationPeriod() does not revert for the valid allocation period: startTime and endTime | Passed | 5M+ |
EwmNftAllowance::updateMaxTotalStakeable() should always revert if the new maxTotalStakeable is not greater than EwmNftAllowance::totalStakeReceived() | Passed | 5M+ |
EwmNftAllowance::updateMaxTotalStakeable() does not revert if the new maxTotalStakeable is greater than EwmNftAllowance::totalStakeReceived() | Passed | 5M+ |
New allocation of NFT tokens should always increase the EwmNftAllowance::totalNftToMint(), EwmNftAllowance::userStakeAmount() and EwmNftAllowance::nftMintAllowance() values | Passed | 5M+ |
Invariant
EwmNftAllowance::setNftExpiryTime()should always revert for the incorrect_nftExpiryTimeTest Result
- Passed
Run Count
- 5M+
Invariant
EwmNftAllowance::setNftExpiryTime()does not revert for the valid_nftExpiryTimeTest Result
- Passed
Run Count
- 5M+
Invariant
EwmNftAllowance::updateAllocationPeriod()should always revert for the incorrect allocation period:startTimeandendTimeTest Result
- Passed
Run Count
- 5M+
Invariant
EwmNftAllowance::updateAllocationPeriod()does not revert for the valid allocation period:startTimeandendTimeTest Result
- Passed
Run Count
- 5M+
Invariant
EwmNftAllowance::updateMaxTotalStakeable()should always revert if the newmaxTotalStakeableis not greater thanEwmNftAllowance::totalStakeReceived()Test Result
- Passed
Run Count
- 5M+
Invariant
EwmNftAllowance::updateMaxTotalStakeable()does not revert if the newmaxTotalStakeableis greater thanEwmNftAllowance::totalStakeReceived()Test Result
- Passed
Run Count
- 5M+
Invariant
- New allocation of NFT tokens should always increase the
EwmNftAllowance::totalNftToMint(),EwmNftAllowance::userStakeAmount()andEwmNftAllowance::nftMintAllowance()values Test Result
- Passed
Run Count
- 5M+
Additional Recommendations
The smart contracts in the scope of this audit could benefit from the introduction of automatic emergency actions for critical activities, such as unauthorized operations like ownership changes or proxy upgrades, as well as unexpected fund manipulations, including large withdrawals or minting events. Adding such mechanisms would enable the protocol to react automatically to unusual activity, ensuring that the contract remains secure and functions as intended.
To improve functionality, these emergency actions could be designed to trigger under specific conditions, such as:
Detecting changes to ownership or critical permissions.
Monitoring large or unexpected transactions and minting events.
Pausing operations when irregularities are identified.
These enhancements would provide an added layer of security, making the contract more robust and better equipped to handle unexpected situations while maintaining smooth operations.