BlockApex (Auditor) was contracted by __PhoenixDAO__ (Client) for the purpose of conducting a Smart Contract Audit/Code Review. This document presents the findings of our analysis which took place on ___28th October 2021____.
Document log: Initial Audit published (4th October 2021) Final Audit published (3rd November 2021)
Scope
The git-repository shared was checked for common code violations along with vulnerability-specific probing to detectmajor issues/vulnerabilities. Some specific checks are as follows:
DAO smart contracts used to maintain onchain proposals. Voting is maintained off-chain. To avoid spams, proposers must have to deposit collateral in $PHNX.
Steps to successful proposal:
1-User submits a proposal and add collateral (Note: collateral is not deducted at at this point)
2-Off-chain voting happens
3-Admins change the status of proposals to Upvote on smart contract.
4-Collateral amount is deducted from the proposer's account.
5-When the status reaches completed, proposer’s collateral is returned back.
Users can earn spot staking rewards by depositing $PHNX for set period of time. If a user unstakes before the set duration, his portion (according to a formula) of staking amount will be burnt. Regardless, the user will get the full rewards at the time of staking.
Users can earn rewards by staking their LP tokens. This staking style is similar to Pancakeswap’s MasterChef.
AUDIT REPORT
Executive Summary
The final analysis indicates that the contracts audited are well-secured. Most of the issues reported in the initial review have been fixed by the client. The contracts were reviewed again in order to ensure maximum security.
Our team performed a technique called “Filtered Audit”, where the contract was separately audited by two individuals. After their thorough and rigorous process of manual testing, an automated review was carried out using Mythril, MythX and Slither.
1. User can withdraw the amount even if the actual deposit didn’t happen
Attack Scenario: User submit a proposal with collateral amount X. As the collateral is deducted when the proposal status is changed to UpVotes by admins. User can withdraw collateral amount of tokens(even if the tokens are not actually deducted). Only checks in place are:
require(!proposalList[proposalId].isClaimed, "Collateral Already Claimed"); require(proposalList[proposalId].proposer == sender, "Only Owner of Proposal can withdraw");
Though this function requires to be called by admins, we suggest that please verify at the backend that the user is eligible to withdraw or not.
Remedy: Verify that the user has his collateral deducted by just adding a flag in proposal struct (collateralDeposited).
Dev’s response: It is only possible when the user has a signature and the user cannot have a signature. Verified at the backend.
2. User can withdraw even if the collateral is returned when the status of proposal is completed
Attack Scenario: When the admin changes the status of the proposal to completed, the contract automatically transfers the collateral back to the proposer. Still there are no checks placed and the collateral is returned back.
Remedy: Add a flag (collateralReturned) in the proposal struct and verify the flag in withdraw.
Dev’s response: It is only possible when the user has a signature and the user cannot have a signature. Verified at the backend.
When users deposit to earn spot staking rewards. StakerData holds the staking information. StakerData needs stakeId to find staking information. stakeId is generated as follows.
When the user unstakes, he has to provide stakeIds to unstake. Now the attacker can use the stakeIds of other users with the following constraint i.e., (sum of withdraw amount from all staking events should be less than attacker's staked amount). Attacker withdraws his amount, but now the original staker does not know his stakeIds.
Note: Although the attacker might have his stakes burnt, the other users will be unable to unstake (at least other users will a have hard time to find stakeIds).
Remedy: We suggest verifying the stakeIds before processing.
2. LPTokens are transferred to $PHNX contract address
The Lp staking smart contract is similar to PCS MasterChef contract. When the user deposits LPs, updatePool method is called. This method transfers some of LP tokens to the $PHNX contract address. This will cause issues when the user tries to unstake their LPs because the contract wouldn’t have enough LPs.
The smart contracts provided by the client for audit purposes have been thoroughly analyzed in compliance with the global best practices till date w.r.t cybersecurity vulnerabilities and issues in smart contract code, the details of which are enclosed in this report.
This report is not an endorsement or indictment of the project or team, and they do not in any way guarantee the security of the particular object in context. This report is not considered, and should not be interpreted as an influence, on the potential economics of the token, its sale or any other aspect of the project.
Crypto assets/tokens are results of the emerging blockchain technology in the domain of decentralized finance and they carry with them high levels of technical risk and uncertainty. No report provides any warranty or representation to any third-Party in any respect, including regarding the bug-free nature of code, the business model or proprietors of any such business model, and the legal compliance of any such business. No third-party should rely on the reports in any way, including for the purpose of making any decisions to buy or sell any token, product, service or other asset. Specifically, for the avoidance of doubt, this report does not constitute investment advice, is not intended to be relied upon as investment advice, is not an endorsement of this project or team, and it is not a guarantee as to the absolute security of the project.
Smart contracts are deployed and executed on a blockchain. The platform, its programming language, and other software related to the smart contract can have its vulnerabilities that can lead to hacks. The scope of our review is limited to a review of the Solidity code and only the Solidity code we note as being within the scope of our review within this report. The Solidity language itself remains under development and is subject to unknown risks and flaws. The review does not extend to the compiler layer, or any other areas beyond Solidity that could present security risks.
This audit cannot be considered as a sufficient assessment regarding the utility and safety of the code, bug-free status or any other statements of the contract. While we have done our best in conducting the analysis and producing this report, it is important to note that you should not rely on this report only - we recommend proceeding with several independent audits and a public bug bounty program to ensure security of smart contracts.
The analysis indicates that the contracts audited are secured and follow the best practices.
Our team performed a technique called “Filtered Audit”, where the contract was separately audited by two individuals. After their thorough and rigorous process of manual testing, an automated review was carried out using Slither, and Manticore. All the flags raised were manually reviewed and re-tested.
Dafi’s “dbridge” enables users to bring their ERC-20 $DAFI tokens across from the Ethereum network to Binance Smart Chain, and vice versa, with aims of making $DAFI available on multiple high-speed and low-cost networks.
In our first iteration, we found 1 critical-risk issue, 4 high-risk issues, 1 medium-risk, 1 low-risk issue and 1 informatory issue. All these issues were refactored and fixes have been made. A detailed report on the first review can be found here.