Introduction
We express our gratitude to the Common Wealth team for the collaborative engagement that enabled the execution of this Smart Contract Security Assessment.
Common Wealth is an investment protocol that allows users to invest their tokens and get rewards.
Document | |
|---|---|
| Name | Smart Contract Code Review and Security Analysis Report for Common Wealth |
| Audited By | Olesia Bilenka |
| Approved By | Przemyslaw Swiatowiec |
| Website | common-wealth.io→ |
| Changelog | 26/09/2024 - Preliminary Report |
| 03/10/2024 - Final Report | |
| Platform | Base |
| Language | Solidity |
| Tags | Non-fungible Token (NFT); Staking; Marketplace; Oracle |
| Methodology | https://hackenio.cc/sc_methodology→ |
Document
- Name
- Smart Contract Code Review and Security Analysis Report for Common Wealth
- Audited By
- Olesia Bilenka
- Approved By
- Przemyslaw Swiatowiec
- Website
- common-wealth.io→
- Changelog
- 26/09/2024 - Preliminary Report
- 03/10/2024 - Final Report
- Platform
- Base
- Language
- Solidity
- Tags
- Non-fungible Token (NFT); Staking; Marketplace; Oracle
- Methodology
- https://hackenio.cc/sc_methodology→
Review Scope | |
|---|---|
| Repository | https://github.com/CommonWealthDAO/commonwealth-contracts/→ |
| Commit | 95f00be |
Review Scope
- Commit
- 95f00be
Audit Summary
The system users should acknowledge all the risks summed up in the risks section of the report
Documentation quality
Functional requirements are provided.
Technical description is provided.
Code quality
The code mostly follows best practices.
The development environment is configured.
Test coverage
Code coverage of the project is 98.14%.
Deployment and basic user interactions are covered with tests.
System Overview
Common Wealth is an investment protocol that allows users to invest their tokens and get rewards by staking or handling NFTs with the following contracts:
UniswapWlthPriceOracle.sol - The contract is designed to provide token prices calculated based on the Oracle library for specific Uniswap pools.
UniswapSwapper.sol - The wrapper for swaps on Uniswap DEX.
WlthFund.sol - The contract that keeps hashed data of proposals driven by discourse and also executes transfers to investees from secondary sales wallet, triggered by CW team or backend along with proper allowance.
ProfitProvider.sol - The contract that handles the profit sent by the project.
OwnablePausable.sol - The contract that handles pause/unpause.
BuybackAndBurn.sol - The contract that automatically buys WLTH tokens from DEX using USDC then burns acquired WLTH.
StateMachine.sol - The contract that restricts unacceptable states.
PerpetualNFT.sol - The NFT contract that reflects perpetual fund investment.
PerpetualFund.sol - The contract that handles perpetual fund logic, raising funds and distributing fees and payouts for investors
WlthBonusStaking.sol - The contract that allows users to stake WLTH for extra WLTH reward. Already deployed and running on production
Marketplace.sol - The contract that handles listing (with edit and cancel option) and selling NFTs functionalities.
Privileged roles
The owner of the UniswapWlthPriceOracle contract can set an observation time and pool addresses.
The owner of the UniswapSwapper contract can set the swap router contract address.
The owner of the ProfitProvider contract can set a minimum profit.
The owner of the BuybackAndBurn contract can set a minimum buyback.
The owner of the PerpetualNFT can add/remove minters, set a perpetual fund, set a metadata name, metadata description, metadata image, all metadata, metadata external URL, minimum value, and marketplace.
The owner of the PerpetualFund contract can set a staking wlth amount, a minimum investment amount, a profit provider, a buyback and burn address, and close funds.
The owner of the WlthBonusStaking contract can set staking schedules.
The owner of the Marketplace contract can add/remove the allowed contracts.
Additionally, the owner of any contracts can pause/unpause the contracts.
Potential Risks
Centralized Control of Minting Process: The token contract’s design allows for centralized control over the minting process, posing a risk of unauthorized token issuance, potentially diluting the token value and undermining trust in the project's economic governance.
Owner's Unrestricted State Modification: The absence of restrictions on state variable modifications by the owner leads to arbitrary changes, affecting contract integrity and user trust, especially during critical operations like minting phases.
Lack of Monitoring on Key Functions: The smart contracts currently lack monitoring for its key functions. Without proper monitoring, it is challenging to detect and respond to unauthorized activities, irregular behaviors, or potential hacking attempts in real-time. This absence of surveillance increases the risk of undetected exploits or issues that could compromise the contract's security and integrity.
Findings
Code ― | Title | Status | Severity | |
|---|---|---|---|---|
| F-2024-6230 | Missing Active Status Check in _cancelListing Function Can Lead to Listing Count Inaccuracy and NFT Locking | fixed | Critical | |
| F-2025-8463 | Incorrect Profit Withdrawal Index Increment Allows Mult iple Withdrawals | fixed | High | |
| F-2024-6234 | Potential Price Manipulation in updateListingPrice Function Can Lead to Frontrunning and Overpayment | fixed | Medium | |
| F-2024-6240 | Missing Slippage Protection in the BuybackAndBurn Contract | fixed | Medium | |
| F-2024-6241 | Insufficient Reward Allocation in WlthBonusStaking Contract Leading to Stake Damages | fixed | Low | |
| F-2024-6229 | Risk of Ownership Control Loss in Owner-Dependent Contracts | mitigated | Low | |
| F-2025-8462 | Unbounded Array Growth Leading to DoS in PerpetualFunds | fixed | Low | |
| F-2024-6242 | Redundant listingId Field in Listing Struct Leads to Unnecessary Gas Usage | fixed | Observation | |
| F-2024-6239 | Redundant Pausable Inheritance | mitigated | Observation | |
| F-2024-6233 | Floating Pragma | fixed | Observation |
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/CommonWealthDAO/commonwealth-contracts/→ |
| Commit | 95f00bef0b9a58c163091ecc715101ff83c65bb0 |
| Whitepaper | https://hackenio.cc/hacken-methodologieshttps://docs.google.com/spreadsheets/d/1qrvf6z_VypMxN6Z7_LnldzNiYiUClaCOnYJ1gc71sFI/edit?gid=1158935575#gid=1158935575→ |
| Technical requirements | https://www.notion.so/common-wealth/→ |
Scope Details
- Commit
- 95f00bef0b9a58c163091ecc715101ff83c65bb0
- Technical requirements
- https://www.notion.so/common-wealth/→
Assets in Scope
Appendix 3. Additional Valuables
Verification of System Invariants
During the audit of Common Wealth, 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, 17 invariants were tested over 80,000 runs. This thorough testing ensured that the system works correctly even with unexpected or unusual inputs.
Invariant | Test Result | Run Count |
|---|---|---|
Marketplace::listNFT should always return Marketplace__ERC721AddressNotAllowed if the newToken is not added using addAllowedContract. The listNFT function should only allow tokens from contracts that are explicitly added to the allowed contract list. | Passed | 80k |
Marketplace::listNFT should always return Marketplace__NFTNotOwnedByMsgSender if the tokenOwner and msg.sender are not the same. The listNFT function should allow only the owner of the token to list it. | Passed | 80k |
Marketplace::listNFT should always return Marketplace__NFTNotApprovedForMarketplaceContract if the token is not approved by approve for the marketplace contract. The listNFT function should only list tokens if they are approved for the marketplace contract. | Passed | 80k |
Marketplace::listNFT should always return Marketplace__NftAlreadyListed if the token is already listed. A token cannot be listed again if it's already listed in the marketplace. | Passed | 80k |
Marketplace::listNFT should always return Marketplace__ZeroPrice if the price of the listed token is zero. The listNFT function should not allow a price of zero. | Passed | 80k |
Marketplace::listNFT should always handle listing of multiple tokens (tokensAmount) for a valid tokenOwner. The marketplace should handle different amounts of token listings (e.g., from 1 to 1000 tokens). | Passed | 80k |
Marketplace::updateListingPrice should always return Marketplace__ZeroPrice if the updated price is zero. The updateListingPrice function should revert if the new price is set to zero. | Failed | 80k |
Marketplace::buyNFT should always return Marketplace__ListingNotActive if the token is not listed. The buyNFT function should only succeed if the token is actively listed. | Passed | 80k |
Marketplace::buyNFT should always return Marketplace__NotEnoughWlthApproved if the buyer’s allowance is less than the price of the listed token. The buyNFT function should revert if the buyer has insufficient allowance. | Passed | 80k |
Marketplace::buyNFT The balanceOf function should return allowance - price after a successful purchase for the buyer’s address. After a successful purchase, the buyer's balance should decrease by the token price. | Passed | 80k |
WlthBonusStaking::stake should always subtract a 1% fee from the staked amount and record the remaining 99% of the tokens under the user’s staked balance. The test should assert that the staked balance for the staker matches the expected amount. | Passed | 80k |
WlthBonusStaking::claimReward should calculate and apply the correct penalty based on the time passed, and the staker should receive the remaining reward after the penalty deduction. | Passed | 80k |
WlthBonusStaking::claimReward should always distribute the reward that remains after the penalty has been deducted. The staked amount should be set to zero after claiming, and the staker’s balance should reflect the correct reward after applying any applicable penalty. | Passed | 80k |
WlthBonusStaking::unstake should always deduct a 1% fee from the staked amount and transfer the remaining 99% to the staker. After unstaking, the staker’s balance should reflect the correct amount after the fee, and the contract should record zero staked tokens for the staker. | Passed | 80k |
WlthBonusStaking::unstake should ensure that the community fund receives both the initial 1% staking fee and the 1% unstaking fee. | Passed | 80k |
PerpetualNFT::mint() should only allow minters who have been registered via addMinter() to mint new tokens. | Passed | 80k |
PerpetualNFT::split() function should revert if the sum of splitValues[] does not equal the original token's value, ensuring value consistency during splits. If the split is successful, new tokens must be created with values exactly matching the specified splitValues[]. | Passed | 80k |
Invariant
Marketplace::listNFTshould always returnMarketplace__ERC721AddressNotAllowedif thenewTokenis not added usingaddAllowedContract. ThelistNFTfunction should only allow tokens from contracts that are explicitly added to the allowed contract list.Test Result
- Passed
Run Count
- 80k
Invariant
Marketplace::listNFTshould always returnMarketplace__NFTNotOwnedByMsgSenderif the tokenOwner and msg.sender are not the same. ThelistNFTfunction should allow only the owner of the token to list it.Test Result
- Passed
Run Count
- 80k
Invariant
Marketplace::listNFTshould always returnMarketplace__NFTNotApprovedForMarketplaceContractif the token is not approved byapprovefor the marketplace contract. ThelistNFTfunction should only list tokens if they are approved for the marketplace contract.Test Result
- Passed
Run Count
- 80k
Invariant
Marketplace::listNFTshould always returnMarketplace__NftAlreadyListedif the token is already listed. A token cannot be listed again if it's already listed in the marketplace.Test Result
- Passed
Run Count
- 80k
Invariant
Marketplace::listNFTshould always returnMarketplace__ZeroPriceif the price of the listed token is zero. ThelistNFTfunction should not allow a price of zero.Test Result
- Passed
Run Count
- 80k
Invariant
Marketplace::listNFTshould always handle listing of multiple tokens (tokensAmount) for a validtokenOwner. The marketplace should handle different amounts of token listings (e.g., from 1 to 1000 tokens).Test Result
- Passed
Run Count
- 80k
Invariant
Marketplace::updateListingPriceshould always returnMarketplace__ZeroPriceif the updated price is zero. TheupdateListingPricefunction should revert if the new price is set to zero.Test Result
- Failed
Run Count
- 80k
Invariant
Marketplace::buyNFTshould always returnMarketplace__ListingNotActiveif the token is not listed. ThebuyNFTfunction should only succeed if the token is actively listed.Test Result
- Passed
Run Count
- 80k
Invariant
Marketplace::buyNFTshould always returnMarketplace__NotEnoughWlthApprovedif the buyer’s allowance is less than the price of the listed token. ThebuyNFTfunction should revert if the buyer has insufficient allowance.Test Result
- Passed
Run Count
- 80k
Invariant
Marketplace::buyNFTThebalanceOffunction should returnallowance - priceafter a successful purchase for the buyer’s address. After a successful purchase, the buyer's balance should decrease by the token price.Test Result
- Passed
Run Count
- 80k
Invariant
WlthBonusStaking::stakeshould always subtract a 1% fee from the staked amount and record the remaining 99% of the tokens under the user’s staked balance. The test should assert that the staked balance for thestakermatches the expected amount.Test Result
- Passed
Run Count
- 80k
Invariant
WlthBonusStaking::claimRewardshould calculate and apply the correct penalty based on the time passed, and the staker should receive the remaining reward after the penalty deduction.Test Result
- Passed
Run Count
- 80k
Invariant
WlthBonusStaking::claimRewardshould always distribute the reward that remains after the penalty has been deducted. The staked amount should be set to zero after claiming, and the staker’s balance should reflect the correct reward after applying any applicable penalty.Test Result
- Passed
Run Count
- 80k
Invariant
WlthBonusStaking::unstakeshould always deduct a 1% fee from the staked amount and transfer the remaining 99% to the staker. After unstaking, the staker’s balance should reflect the correct amount after the fee, and the contract should record zero staked tokens for the staker.Test Result
- Passed
Run Count
- 80k
Invariant
WlthBonusStaking::unstakeshould ensure that the community fund receives both the initial 1% staking fee and the 1% unstaking fee.Test Result
- Passed
Run Count
- 80k
Invariant
PerpetualNFT::mint()should only allow minters who have been registered viaaddMinter()to mint new tokens.Test Result
- Passed
Run Count
- 80k
Invariant
PerpetualNFT::split()function should revert if the sum ofsplitValues[]does not equal the original token's value, ensuring value consistency during splits. If the split is successful, new tokens must be created with values exactly matching the specifiedsplitValues[].Test Result
- Passed
Run Count
- 80k