The audit makes no statements or warranties about utility of the code, safety of the code, suitability of the business model, investment advice, endorsement of the platform or its products, regulatory regime for the business model, or any other statements about fitness of the contracts to purpose, or their bug free status. The audit documentation is for discussion purposes only. The information presented in this report is confidential and privileged. If you are reading this report, you agree to keep it confidential, not to copy, disclose or disseminate without the agreement of Client. If you are not the intended recipient(s) of this document, please note that any disclosure, copying or dissemination of its content is strictly forbidden.
A group of auditors are involved in the work on the audit. The security engineers check the provided source code independently of each other in accordance with the methodology described below:
- Project documentation review.
- General code review.
- Reverse research and study of the project architecture on the source code alone.
- Build an independent view of the project's architecture.
- Identifying logical flaws.
- Manual code check for vulnerabilities listed on the Contractor's internal checklist. The Contractor's checklist is constantly updated based on the analysis of hacks, research, and audit of the clients' codes.
- Code check with the use of static analyzers (i.e Slither, Mythril, etc).
Eliminate typical vulnerabilities (e.g. reentrancy, gas limit, flash loan attacks etc.).
- Detailed study of the project documentation.
- Examination of contracts tests.
- Examination of comments in code.
- Comparison of the desired model obtained during the study with the reversed view obtained during the blind audit.
- Exploits PoC development with the use of such programs as Brownie and Hardhat.
Detect inconsistencies with the desired model.
- Cross check: each auditor reviews the reports of the others.
- Discussion of the issues found by the auditors.
- Issuance of an interim audit report.
- Double-check all the found issues to make sure they are relevant and the determined threat level is correct.
- Provide the Client with an interim report.
- The Client either fixes the issues or provides comments on the issues found by the auditors. Feedback from the Customer must be received on every issue/bug so that the Contractor can assign them a status (either "fixed" or "acknowledged").
- Upon completion of the bug fixing, the auditors double-check each fix and assign it a specific status, providing a proof link to the fix.
- A re-audited report is issued.
- Verify the fixed code version with all the recommendations and its statuses.
- Provide the Client with a re-audited report.
- The Customer deploys the re-audited source code on the mainnet.
- The Contractor verifies the deployed code with the re-audited version and checks them for compliance.
- If the versions of the code match, the Contractor issues a public audit report.
- Conduct the final check of the code deployed on the mainnet.
- Provide the Customer with a public audit report.
All vulnerabilities discovered during the audit are classified based on their potential severity and have the following classification:
| Severity | Description |
|---|---|
| Critical | Bugs leading to assets theft, fund access locking, or any other loss of funds. |
| High | Bugs that can trigger a contract failure. Further recovery is possible only by manual modification of the contract state or replacement. |
| Medium | Bugs that can break the intended contract logic or expose it to DoS attacks, but do not cause direct loss funds. |
| Low | Bugs that do not have a significant immediate impact and could be easily fixed. |
Based on the feedback received from the Customer regarding the list of findings discovered by the Contractor, they are assigned the following statuses:
| Status | Description |
|---|---|
| Fixed | Recommended fixes have been made to the project code and no longer affect its security. |
| Acknowledged | The Customer is aware of the finding. Recommendations for the finding are planned to be resolved in the future. |
The audited project consist of two repositories.
p2p-eth2-depositor- this smart contract allows depositing up to 100 ETH2 validator nodes at one transaction. After being funded, the validator nodes will generate fees.eth-staking-fee-distributor- these smart contracts manage the distribution of received fees among the service, the client, and (optionally) the referral.
| Title | Description |
|---|---|
| Client | P2P.ORG (P2P Staking) |
| Project name | ETH2 Depositor & ETH Staking Fee Distributor |
| Timeline | 12 Dec 2022 - 15 Dec 2022 |
| Number of Auditors | 3 |
| Date | Commit Hash | Note |
|---|---|---|
| 12.12.2022 | 9ad1803294a968e9bc4ce9a24e37b0f312a07d0e | initial commit (eth depositor) |
| 12.12.2022 | 4bac8f636d7ba14244612bcfb9e85f338feba6e3 | initial commit (fee distributor) |
| 23.12.2022 | 99c38b3e025cff1659755b279d3e39142ffd9713 | commit with fixes (eth depositor) |
| 23.12.2022 | 06f029bb27ef97d9955d83c8d92fbb0c6468536e | commit with fixes (fee distributor) |
The audit covered the following files:
| Severity | # of Findings |
|---|---|
| CRITICAL | 0 |
| HIGH | 0 |
| MEDIUM | 2 |
| LOW | 4 |
We discovered 2 medium, and 4 low severity issues during the audit process. All issues were confirmed and fixed by the client.
| File name | Contract deployed on mainnet |
|---|---|
| P2pEth2Depositor.sol | 0x4ca21e4d3a86e7399698f88686f5596dbe74adeb |
| FeeDistributorFactory.sol | 0xba1daf11b323dd8c8f84931220918689aaad041a |
| OwnableAssetRecoverer.sol | 0xba1daf11b323dd8c8f84931220918689aaad041a |
| OwnableWithOperator.sol | 0xba1daf11b323dd8c8f84931220918689aaad041a |
| OwnableTokenRecoverer.sol | 0xba1daf11b323dd8c8f84931220918689aaad041a |
| AssetRecoverer.sol | 0xba1daf11b323dd8c8f84931220918689aaad041a |
| TokenRecoverer.sol | 0xba1daf11b323dd8c8f84931220918689aaad041a |
| OwnableBase.sol | 0xba1daf11b323dd8c8f84931220918689aaad041a |
| Ownable2Step.sol | 0xba1daf11b323dd8c8f84931220918689aaad041a |
| FeeDistributor.sol | 0xe2c725b9ed614be2864892f3b8727ce467bc5954 |
| OwnableTokenRecoverer.sol | 0xe2c725b9ed614be2864892f3b8727ce467bc5954 |
| TokenRecoverer.sol | 0xe2c725b9ed614be2864892f3b8727ce467bc5954 |
| OwnableBase.sol | 0xe2c725b9ed614be2864892f3b8727ce467bc5954 |
Not found
Not found
The transfer of the ownership can be performed to an arbitrary account whether or not it is controlled by the current authority. After the transfer to an uncontrolled account, the ownership will be lost.
We recommend using a two-step ownership transfer procedure, including the confirmation (accept) of the ownership transfer.
If one of the fee receivers can't accept ether (i.e the transaction will revert or consume too much gas), no ether can be withdrawn. Although a new instance can be redeployed, some ether may stuck in the recent instance.
We recommend implementing the functionality to migrate from a stuck FeeDistributor instance to a new one.
The depositContract variable is initialized only in the constructor and not changed any more.
https://github.com/p2p-org/p2p-eth2-depositor/blob/main/src/P2pEth2Depositor.sol#L15
The current variable type (mutable) is designed for variables that are supposed to be changed during lifecycle.
Reading such variables consumes more gas than reading immutable variables.
We recommend adding the immutable keyword to the depositContract variable.
At line
https://github.com/p2p-org/p2p-eth2-depositor/blob/9ad1803294a968e9bc4ce9a24e37b0f312a07d0e/src/P2pEth2Depositor.sol#L10
P2pEth2Depositor inherits ReentrancyGuard, but the functions of this contract are not used.
Inheritance of unnecessary code generally increases the likelihood of vulnerabilities.
We recommend removing the ReentrancyGuard inheritance.
At line https://github.com/p2p-org/p2p-eth2-depositor/blob/9ad1803294a968e9bc4ce9a24e37b0f312a07d0e/src/P2pEth2Depositor.sol#L83 excess transformation can be removed.
It is recommended to replace the line with
depositContract.deposit{value: collateral}(
pubkeys[i],
withdrawal_credentials[i],
signatures[i],
deposit_data_roots[i]
);
If recipient in s_referrerConfig is a null address, then there is no need to calculate referrerAmount, because referrerAmount will always be 0.
Also, referrerAmount will be redundant for serviceAmount. Additionaly, we can bypass calling the sendValue with zero value and null address.
https://github.com/p2p-org/eth-staking-fee-distributor-contracts/blob/4bac8f636d7ba14244612bcfb9e85f338feba6e3/contracts/feeDistributor/FeeDistributor.sol#L256
https://github.com/p2p-org/eth-staking-fee-distributor-contracts/blob/4bac8f636d7ba14244612bcfb9e85f338feba6e3/contracts/feeDistributor/FeeDistributor.sol#L244
https://github.com/p2p-org/eth-staking-fee-distributor-contracts/blob/4bac8f636d7ba14244612bcfb9e85f338feba6e3/contracts/feeDistributor/FeeDistributor.sol#L247
We recommend adding a null check for recipient in s_referrerConfig in the withdraw function and update logic for gas saving.
MixBytes is a team of blockchain developers, auditors and analysts keen on decentralized systems. We build opensource solutions, smart contracts and blockchain protocols, perform security audits, work on benchmarking and software testing solutions, do research and tech consultancy.