Ever Steak A trustworthy and responsible validator. 625k+ user over 70 years old A blockchain network created by engineers for the entire community in 2018.
Everstake 0.1+ ETH staking solution is a protocol that allows users to deposit amounts less than 32 ETH. When a user’s deposit exceeds 32 ETH, a new validator will be created and the user will be eligible to receive rewards. Staking rewards are automatically re-staked and increase the user’s pool stake.
Everstake worked with Ackee Blockchain to conduct a security review of the Everstake Protocol (Revisions 1.0 and 2.0). 20 engineering work in the period between July 3 and August 29, 2023.
Everstake also worked with Ackee Blockchain to conduct a redaction review of the second audit revision of the commit. 38970a6.
methodology
Revision 1.0
We use static analysis tools, namely I woke up. We then took a closer look at the logic of the contract. For testing and fuzzing we use I woke up Test framework.
During the review process, we paid special attention to the following:
- Verify that the system’s accounting arithmetic is correct
Test that the unstaking path does not revert - Test whether users cannot withdraw more than they deposited (+ rewards)
- Validator Management Analysis
- Detect possible ETH call re-entrancy in your code
- Ensure access controls are neither too relaxed nor too strict
- Test whether rewards are distributed according to the user’s stake
- Analyze whether the contract uses appropriate data structures to store deposits, withdrawals, etc.
- Upgradability pattern analysis (storage conflicts, access control, etc.)
- Analyze whether your withdrawal credentials were created correctly
- I’m looking for common problems like data validation.
Revision 2.0
We followed the methodology established in previous revisions.
- We wrote additional tests focused on manually reviewing all changes. It happened.
- We have written a new simple differential fuzz test for the quick sort function and made it available here: Awakening appendix.
- We also used I woke up It is primarily intended for static analysis, useful for reentrant analysis.
During the review process, we had similar goals as previous revisions. We also focused on:
- Verify that all modifications were applied correctly
- Withdrawal of extreme amounts (e.g. very small value, same value as all stocks, etc.)
- Issuance of shares and subsequent distribution of compensation
- Examining integer division-based precision loss introduced in amount-quotient conversion
- Review view features (mainly _simulateAutocompound)
- Review the new ordering logic of the validator
- Analysis of trading orders and forward trading opportunities
- Review new upgradeability patterns.
range
Revision 1.0
An audit was performed at commit time. 60688fc, The scope was full. contract folder:
contracts/ ├── Accounting.sol ├── AutocompoundAccounting.sol ├── Governor.sol ├── Pool.sol ├── RewardsTreasury.sol ├── TreasuryBase.sol ├── WithdrawTreasury.sol ├── Withdrawer.sol ├── common │ └── Errors.sol ├── interfaces │ ├── IAccounting.sol │ ├── IDepositContract.sol │ ├── IPool.sol │ ├── IRewardsTreasury.sol │ └── ITreasuryBase.sol ├── lib │ ├── UnstructuredRefStorage.sol │ └── UnstructuredStorage.sol
├── structs │ ├── ValidatorList.sol │ └── WithdrawRequests.sol └── utils ├── Math.sol └── OwnableWithSuperAdmin.sol
Revision 2.0
Review completed at commit time. 35f9b56 The files in scope were the same as in the previous audit. Since the last audit, 45 new commits have been created, many of which are fix and refactoring commits. The most notable changes are:
- Added upgrade possibility to treasury
- Make exchanges optional
- Modify the ordering logic of the validator
- Added gas optimization
- Improved variable naming and added descriptions
- Resolve any issues discovered in previous audits.
Revision 2.1
Performed a revision review of the second audit revision of the commit. 38970a6.
result
Here we have our result.
Revision 1.0
critical severity
No critical severity issues were found.
Severity High
H1: _simulateAutocompound is reversible.
H2: DoS due to 0 pending deposits
H3: Partial DoS due to exchange
H4: DoS due to underflow
medium severity
M1: You are missing the whenWithdrawActive modifier.
M2: Deposited BalanceOf revert call
low severity
L1: The array of withdrawal requests increases monotonically.
L2: Lack of second level role transfer
L3: A terminated validator can be reverted.
L4: Validator replacement lacks validation.
L5: Verification of owner of treasury
L6: Data validation in initialization function
L7: Incorrect return value from _simulateAutocompound
L8: Upgradeable contract creator without initialization
L9: Insufficient data verification when creating contracts
warning severity
W1: How to use Solc optimizer
W2: Dead code in _autoCompoundUserBalance
W3: Unchecked return from _update
W4: Storage location lacks contract prefix
W5: Pool fees can be set very high.
Information Severity
I1: Used library
I2: Comparison with role external modifiers
I3: The function always returns true.
I4: Lack of login configurators
I5: Code and comments mismatch
I6: lack of documentation
revision 2.0
critical severity
No critical severity issues were found.
Severity High
H5: If you withdraw the AutocompoundBalanceOf amount, it will be returned to you.
medium severity
M3: simulateAutocompound only checks balance differences.
low severity
L10: Pending deposits cannot be withdrawn.
L11: Insufficient call to disableInitializers().
L12: lack of week 0 confirmation in simulateAutocompound
L13: FeeBalance lacks 0 week confirmation
warning severity
W6: Withdrawals may be returned 1wei more than requested.
W7: Withdrawal reinstatement due to rounding
W8: unstakePending and activateBalance can be reverted due to poor timing.
Information Severity
I7: Code duplication for ownership
I8: Typos in code and comments
I9: Array length verification
conclusion
Revision 1.0
Here are the results of our review: 26 survey resultsup to information to High Seriousness. The highest severity issues are related to: denial of service and Can’t see protocol status Due to return of underflow.
Overall, we recommend that you do not deploy the current version of the protocol. During the audit, we discovered several issues that caused the protocol to revert, even though the state was only achieved through normal, non-malicious transactions.
This means that the protocol has not been sufficiently tested. We also found that documentation was lacking, so we dedicated a separate document. information problem Therefore.
At the same time, we want to acknowledge that the development team may have discovered some issues independently of our review (i.e., issues discovered by both teams independently of each other). H3. Additionally, during our audit, we observed a number of clever design decisions. swimming pool and Accounting contract.
However, due to the number of issues, including high-severity issues, some work must be done to make the protocol production-ready.
The audit process was slowed by the high number of issues, lack of documentation, and the fact that protocols were reverted in certain scenarios. Therefore, we cannot be sure that the protocol will be completely secure after the fixes are applied. It is recommended to run shorter audit rounds again to ensure the security of the protocol.
Revision 2.0
The results of the second review are as follows: 12 survey resultsup to information to High Seriousness. The most severe issues related to: integer division based error, In certain protocol states, withdrawal is reversed thus caused Temporary locking of user funds.
In the second revision, the code quality has improved significantly. The code was easier to read (mainly because we used better names for variables) and the documentation was better. Almost all problems from previous revisions have been fixed.
Based on observations made during the review process, We recommend focusing on the following high-level goals::
- Documentation is still lacking and could be improved.
- This raises another rounding-based issue, so fuzzing is recommended.
- It is a protocol that ensures that other subtle errors do not occur.
- Another bug was discovered. _simulateAutomatic synthesis To enable the feature, we recommend that you rethink your approach to creating simulations and use a more organized and structured approach.
- Avoid overly complex and over-engineered solutions, such as rearranging and replacing validators. These optimizations are generally not worth it in the long run.
- Please fix any reported issues.
Revision 2.1
We believe all issues have been resolved correctly. we are H5 rounding issue There are no other rounding issues causing bounces in the default user flow. However, it is still recommended to fuzz a protocol to analyze its behavior under random scenarios and protocol states.
Ackee blockchain is full Ever Steak You can find the audit report with a more detailed description of all findings and recommendations. here.
We were happy to give our thanks. Ever Steak We look forward to working with them again.