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 the 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. |
METH is a liquid staking protocol that allows users to stake their ETH and receive staking rewards generated from validators' activity in the consensus layer and additional rewards from the execution layer. When users deposit their ETH, they receive METH as proof of deposit. The balance of METH remains the same during staking, but its exchange rate to ETH increases with new rewards and decreases with slashings. The protocol logic for unstake requests works in the following way. If a user unstake some METH and after that, a slashing event occurs, then their's unstake request will be canceled, and accrued rewards for the time that the user was waiting for unstake will be accounted on their balance.
| Title | Description |
|---|---|
| Client | Mantle Network |
| Project name | METH |
| Timeline | October 04 2023 - November 21 2023 |
| Number of Auditors | 3 |
| Date | Commit Hash | Note |
|---|---|---|
| 04.10.2023 | bd15a96aee7df0fc0566d662df0cf50ff2619d31 | Commit for the audit (mntEth) |
| 26.10.2023 | 93a55d8a3e9dfdb40ebd95e1e350af9d0c821883 | Commit for the reaudit (mntEth) |
| 06.11.2023 | 042cddc6f7dfdd1d76036b2c50edb3d859769a98 | Commit for the audit (contracts) |
| 16.11.2023 | 41cefefb4acce2b3763ba32d76b95518594f9653 | Commit for the audit (METHL2) |
| 21.11.2023 | b05cf1aa4c206d9c0c65559915a9199bd509c009 | Commit with update (METHL2) |
The audit covered the following files:
| Severity | # of Findings |
|---|---|
| CRITICAL | 0 |
| HIGH | 3 |
| MEDIUM | 4 |
| LOW | 7 |
During the audit process 3 HIGH, 4 MEDIUM, and 7 LOW severity findings were spotted. After working on the reported findings, all of them were acknowledged or fixed by the client. The client acknowledged a big chunk of the findings because the modifications that admins will make will be thoroughly checked before applying them. The Mantle team clarified that all updates will be made via multisigs with an appropriate amount of checks, which can be suitable for the protocol growing stage (when the TVL of the protocol is not very high and some business logic can be updated to add new necessary functionalities). But for the cases when protocol holds a huge TVL, we recommend minimizing protocol updates and using an architecture that requires minimum input from trusted parties (e.g., ZK-oracles for reporting protocol state from the beacon chain, additional checks in modification function that guarantees modification correctness, etc.).
Not found
ACKNOWLEDGED
Lack of sanity checks after record update can lead to a situation where new oracle reports will not pass checks because of incorrect values in previously updated reports https://github.com/TwoFiftySixLabs/mntEth/blob/bd15a96aee7df0fc0566d662df0cf50ff2619d31/src/Oracle.sol#L290.
The thing is that the modifier can update the last record with incorrect values, so new oracle reports will not pass sanity checks in the receiveRecord function. Additionally, the ORACLE_MODIFIER_ROLE holder can change the last record, bypassing the sanityCheckUpdate(). As a result, the last record can become incorrect, with an arbitrary value of currentTotalValidatorBalance and, to some degree, cumulativeProcessedDepositAmount. Consequently, the holder of the ORACLE_MODIFIER_ROLE can control Staking.totalControlled(): https://github.com/TwoFiftySixLabs/mntEth/blob/bd15a96aee7df0fc0566d662df0cf50ff2619d31/src/Staking.sol#L574
So, mETH exchange rate can be changed in one transaction. mETH can be minted at a reduced rate and sent to unstaking at a heightened rate. The finding is classified as HIGH severity since after an incorrect update, only the admin can fix the contract to continue receiving new reports.
We recommend adding sanity checks on record modification:
sanityCheckUpdate(_records[idx-1], record);
sanityCheckUpdate(record, _records[idx+1]);
This is by design. It is important that the admin can modify records, and the records may have values which are out of the bounds of the checks in recovery cases. There are validity checks for strict invariants of the protocol which cannot be violated, even by the modification function.
ACKNOWLEDGED
There is a chance that some of the oracle addresses will be compromised. After key leakage, the oracle address should be removed from the quorum, but if the malicious user sends a report before being excluded from the quorum, then their report will be accounted for in the quorum https://github.com/TwoFiftySixLabs/mntEth/blob/bd15a96aee7df0fc0566d662df0cf50ff2619d31/src/OracleQuorumManager.sol#L184-L186. This vulnerability is classified as HIGH because after quorum reduction, it is easier for the malicious user to reach quorum (https://github.com/TwoFiftySixLabs/mntEth/blob/bd15a96aee7df0fc0566d662df0cf50ff2619d31/src/OracleQuorumManager.sol#L132-L133) and the compromised record will be valid.
We recommend setting reporterRecordHashesByBlock to 0 after the SERVICE_ORACLE_REPORTER role modification in the contract.
In practice, the absolute threshold parameter will require a majority of honest oracles to report the same hash.
3. The latestCumulativeETHRequested state variable can be zeroed inside the cancelUnfinalized function
Fixed in https://github.com/TwoFiftySixLabs/mntEth/commit/93a55d8a3e9dfdb40ebd95e1e350af9d0c821883
There is an issue at the line: https://github.com/TwoFiftySixLabs/mntEth/blob/bd15a96aee7df0fc0566d662df0cf50ff2619d31/src/UnstakeRequestsManager.sol#L269. It is possible that latestRemainingRequest was a claimed request, so that latestRemainingRequest.cumulativeETHRequested equals to zero. In such a case the latestCumulativeETHRequested value is changed to zero, which leads to two problems. The allocatedETHSurplus function defined at line https://github.com/TwoFiftySixLabs/mntEth/blob/bd15a96aee7df0fc0566d662df0cf50ff2619d31/src/UnstakeRequestsManager.sol#L332 would return a bigger value than it should (as allocatedETHForClaims is a cumulative variable), which will lead to revert inside the withdrawAllocatedETHSurplus function as it will attempt to transfer more ETH than it is stored on the contract.
The second issue is related to the unstake requests queue. There is a check at line https://github.com/TwoFiftySixLabs/mntEth/blob/bd15a96aee7df0fc0566d662df0cf50ff2619d31/src/UnstakeRequestsManager.sol#L199.
It ensures that users can claim in a first-come-first-serve manner, but if there are new requests created after zeroed latestCumulativeETHRequested, then users who joined the queue later would be able to claim before someone who had joined the queue earlier.
This finding is classified as HIGH since it leads to funds stuck on the UnstakeRequestsManager (withdrawAllocatedETHSurplus would be locked) and unfair claims of user requests. Those issues can be resolved only by redeploying the contract.
We recommend reducing the latestCumulativeETHRequested value only by the sum of the ethRequested fields of cancelled requests.
Great find.
ACKNOWLEDGED
There is an issue at line https://github.com/TwoFiftySixLabs/mntEth/blob/bd15a96aee7df0fc0566d662df0cf50ff2619d31/src/UnstakeRequestsManager.sol#L361. It is possible for an attacker to front-run numberOfBlocksToFinalize extension and claim an unstake request which would have gotten unfinalized after such extension. If there are some unfinalized requests before that claimed request, then they would get uncancellable due to the check at line https://github.com/TwoFiftySixLabs/mntEth/blob/bd15a96aee7df0fc0566d662df0cf50ff2619d31/src/UnstakeRequestsManager.sol#L243 (which will stop at the request claimed by an attacker).
This issue is classified as MEDIUM as it can lead to the inability to cancel unfinalized requests.
We recommend adding a special argument to the cancelUnfinalizedRequests function which would help to skip such claimed unstake requests if unfinalized requests are lying before them in the queue.
The issue presupposes that the setter for this field is intended to allow an admin to change finalization of requests which were already made, but this is not the case. Once requests become final, there is no intention to ever cancel them. Hence, a user front-running this transaction has no effect on the protocol. This function should only be used in normal operation, not when the protocol has experienced an event which requires cancellation.
ACKNOWLEDGED
If numberOfBlocksToFinalize is set higher than the end block number of the latest record in oracle, then UnstakeRequestsManager._isFinalized() returns false for each claimed request
https://github.com/TwoFiftySixLabs/mntEth/blob/bd15a96aee7df0fc0566d662df0cf50ff2619d31/src/UnstakeRequestsManager.sol#L377.
As a result, UnstakeRequestsManager.cancelUnfinalizedRequests() will revert on such requests (due to an attempt to make a transfer to address(0)):
https://github.com/TwoFiftySixLabs/mntEth/blob/bd15a96aee7df0fc0566d662df0cf50ff2619d31/src/UnstakeRequestsManager.sol#L275.
Furthermore, this can lead to other breaches in the logic of UnstakeRequestsManager.
We recommend setting a reasonable upper bound for numberOfBlocksToFinalize in UnstakeRequestsManager.setNumberOfBlocksToFinalize(). We also recommend not completely deleting the claimed request but only marking it as claimed.
We assume a competent and sane operator that does not set this number to millions of blocks. If that did happen, the fix would simply be to set it back.
ACKNOWLEDGED
withdrawalWallet can be updated in the Staking contract https://github.com/TwoFiftySixLabs/mntEth/blob/bd15a96aee7df0fc0566d662df0cf50ff2619d31/src/Staking.sol#L709 but it cannot be updated in the ReturnsAggregator contract. Moreover, if the admin decides to update withdrawalWallet, it can break the protocol and require updating implementation for the ReturnsAggregator contract.
We recommend updating the architecture of the ReturnsAggregator contract so it can process with 2 different withdrawal wallet addresses.
We chose to initially implement support for a single withdrawal wallet for simplicity. In the rare case which this needs to change, the operator will need to upgrade the ReturnsAggregator contract to add support for multiple withdrawal wallets.
ACKNOWLEDGED
A user that wants to receive unstaked ETH without losses can DoS cancelUnfinalizedRequests https://github.com/TwoFiftySixLabs/mntEth/blob/bd15a96aee7df0fc0566d662df0cf50ff2619d31/src/UnstakeRequestsManager.sol#L240 by simply creating dozens of unstake requests. This will require only additional gas costs for sending transactions, all METH that will be blocked on the UnstakeManager during DoS will be returned to the user. Additional unstake requests will require admin to remove more requests than they expected, leading to out-of-gas cases or not all requests will be removed.
We recommend calling cancelUnfinalizedRequests only via private pools so users cannot front-run this call.
This is mitigated by the ability to pause unstake requests when cancelling. In practice, the contract will always be in a paused state for this function execution, as cancellation is an exceptional case which is only needed when there is a major slashing event (which pauses the protocol automatically). A private mempool can also be used.
Fixed in https://github.com/TwoFiftySixLabs/mntEth/commit/93a55d8a3e9dfdb40ebd95e1e350af9d0c821883
Execution layer rewards can be transferred to the ReturnsReceiver. Thus, they should be monitored by an external service to ensure that all EL rewards were sent by validator https://github.com/TwoFiftySixLabs/mntEth/blob/bd15a96aee7df0fc0566d662df0cf50ff2619d31/src/ReturnsAggregator.sol#L124.
We recommend using an external service to monitor EL rewards.
This check is implemented by our guardian service to ensure that rewards are received.
ACKNOWLEDGED
There is a chance that the UnstakeRequestCreated event will be emitted several times with the same requestId https://github.com/TwoFiftySixLabs/mntEth/blob/bd15a96aee7df0fc0566d662df0cf50ff2619d31/src/UnstakeRequestsManager.sol#L175-L177.
We recommend accounting for this fact in off-chain services or deleting requests during cancellation and not removing them from the list.
This is by design. Off-chain indexing services account for this.
ACKNOWLEDGED
A topUp call can dramatically increase the exchange rate, so users that will front-run this call will "steal" some rewards https://github.com/TwoFiftySixLabs/mntEth/blob/bd15a96aee7df0fc0566d662df0cf50ff2619d31/src/Staking.sol#L523-L525.
We recommend using private mem pools for this call.
The operator can choose to pause staking or use a private mempool as suggested.
Fixed in https://github.com/TwoFiftySixLabs/mntEth/commit/93a55d8a3e9dfdb40ebd95e1e350af9d0c821883
There is an issue at the line: https://github.com/TwoFiftySixLabs/mntEth/blob/bd15a96aee7df0fc0566d662df0cf50ff2619d31/src/UnstakeRequestsManager.sol#L269. latestCumulativeETHRequested gets updated even if there were no canceled requests and the numCancelled variable equals to zero.
We recommend adding a check for the numCancelled value before updating the latestCumulativeETHRequested variable.
comment here
ACKNOWLEDGED
Currently, the oracle service doesn't process the case when a different report was sent by a malicious user from the oracle address to the OracleQuorum contract. It is better to catch the situation when OracleQuorum already has non-zero hash for the same endBlock. In this case, it is better to alert immediately and pause the Oracle contract.
We recommend adding logging and alerting for this case.
It is expected that oracles may need to resubmit reports in some cases. Due to the off-chain services being designed to be stateless, it is difficult to detect cases where a re-report is due to compromise rather than re-computation. We agree that these cases should be monitored and investigated.
Fixed in https://github.com/TwoFiftySixLabs/mntEth/commit/93a55d8a3e9dfdb40ebd95e1e350af9d0c821883
An oracle can send a report for any block range, including old blocks and new blocks https://github.com/TwoFiftySixLabs/mntEth/blob/bd15a96aee7df0fc0566d662df0cf50ff2619d31/src/OracleQuorumManager.sol#L182.
We recommend adding an alert if the quorum was reached more than once for a specific block.
Reaching quorum twice may be needed in some cases, but we agree that this is extremely unlikely and so there should be alerting for this case.
ACKNOWLEDGED
Record modification cannot lead to rewards decreasing because it is impossible to receive more rewards than ReturnsReceivers have https://github.com/TwoFiftySixLabs/mntEth/blob/bd15a96aee7df0fc0566d662df0cf50ff2619d31/src/Oracle.sol#L298-L303. If rewards were decreased during modification, it should be fixed by the admin, which will lead to new rewards that will not be on the contract. If the admin makes a mistake during modification, then the protocol will need to send additional funds to the Receivers' contracts to modify the record back.
We recommend thoroughly checking record modification because incorrect updating will require admins to top-up the Return Aggregator contract.
When needed, all reward modification is computed automatically by code.
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.