Omniscia Moby Audit
Options Market Implementation Security Audit
Audit Report Revisions
Commit Hash | Date | Audit Report Hash |
---|---|---|
82212ec6e5 | March 19th 2024 | 73e7a4b6c2 |
82212ec6e5 | March 19th 2024 | 09852a4aa6 |
b02fae335f | April 15th 2024 | f46e6c60d0 |
a8720219a6 | May 28th 2024 | 0fe13571b7 |
a95db4124c | June 20th 2024 | ff7a72d250 |
Audit Overview
We were tasked with performing an audit of the Moby codebase and in particular their Options Market Implementation module.
Over the course of the audit, we were able to identify a vulnerability that compromises the queue system and thus core functionality of the protocol, as well as a double-spend type of flaw in the queue's processing mechanism.
In addition to the above, misbehaviours were observed in the RewardTracker
implementation as it appears to not properly track the asset transfers that it performs.
The technical due diligence report shared with us as part of the engagement appears to not account for EVM-constraints, such as block gas limits, and focuses instead on transaction processing capabilities.
The system makes use of array-based entries throughout, and the VaultUtils
implementation in particular was assessed to be capable of breaching those block limitations under the right circumstances.
The documentation of the project is disproportionate to its complexity, and the security effort was performed to the extent possible based on the available data at the time of the audit.
All informational
exhibits identified during the audit that are style-related merely serve as recommendations, and do not pose any security risk to the protocol in any shape or form.
Furtheremore, a total of 35 informational
findings identified in the audit are derived from the GMX library the project utilizes and are thus not related to its own code.
The Moby team attempts to build an innovative protocol and in this effort has produced a significantly novel but convoluted system.
Throughout the audit effort, we were able to identify a high number of findings that relate to the code's style, inefficiencies, and overall structure.
Overall, we believe the codebase needs to be refactored, rewritten, and/or re-explored due to the following points:
- Logic implementations are concentrated within singleton contracts instead of taking advantage of composition, leading to contracts with a significantly high line count
- Certain contract flows contain upward to 6 inward calls all of which will rely on validation performed at one of the upper levels which may not always be true and thus results in a difficult-to-maintain system
- Fund flows are inefficient and performed at different steps of a nested call with different recipients than their intended final destinations, rendering their validation difficult
- Code repetition is observed throughout, oftentimes implementing the exact same statements at different instances of the codebase which increases the complexity of the project with no benefit
- Programming inefficiencies have been observed throughout the system, including but not limited to: local variable re-assignments with interchangeable use, unnecessary variable abbreviations, unreachable statements
- Mathematical formulae expected to be adhered to by the system lack citations or documentation in the code itself and have minimal documentation in off-code resources
- Documentation throughout the codebase is minimal, at-times contradictory, and inadequate indicating a "black box" approach in the development phase
- No compilation framework has been specified, and no tests can be observed in the codebase
- TODO comments were identified throughout the codebase and have been marked as informational issues but need to be resolved
Once the above points have all been rectified, our formal recommendation is to perform a secondary security review of the system as we do not consider it production-ready at this point.
We advise the Moby team to closely evaluate all minor-and-above findings identified in the report and promptly remediate them as well as consider all optimizational exhibits identified in the report.
Post-Audit Conclusion
The Post-Audit Conclusion
chapters of this audit report represent a historical analysis of the codebase on each iteration our audit team engaged in, ordered from oldest to most recent. To evaluate the state of the latest version of the codebase, kindly consult the last Post-Audit Conclusion
chapter.
The Moby team iterated through all findings within the report and provided us with a revised commit hash to evaluate all exhibits on.
We evaluated all alleviations performed by Moby and have identified that certain exhibits have not been adequately dealt with.
The following finding's alleviation introduced a major vulnerability to the codebase: VUS-03M
Additionally, we advise the Moby team to revisit the following exhibits as the remediation that the Moby team shared is insufficient: RTR-01M
, RTR-02M
The following exhibit concerns a misbehaviour that is bound to arise in the future, and we strongly advise the Moby team to reassess it: VTL-04S
Concluding the remediation review, the following informational
findings have been partially addressed signalling intent to alleviate them and should thus be revisited: FDR-01S
, OMR-02S
, RTR-01S
, SPF-03S
, VUS-03S
We would like to outline that all base implementation initializations have been acknowledged by the Moby team which we consider a deviation from best practices. We strongly advise all logic implementations to be initialized during their constructor, preventing their logic deployments from being initialized distinctly from their proxy implementations.
A new Referral
contract has been introduced in the latest commit hash that was supplied to us, and has been reviewed as part of the alleviation evaluation round.
Within the contract, we have identified several issues including a potential design flaw as well as multiple major vulnerabilities grouped under a single root cause.
Specifically, the administrative functions of the Referral
contract apply no access control permitting the various discount rates to be arbitrarily configured. In detail, the following functions lack access control:
We advise access control to be imposed to ensure that the promotional discount rates are configured by the Moby team.
From a design perspective, the contract will offer a discount to a user who has zero referrals and has simply specified their referrer. This contradicts the expected purpose of a referral system whereby users with more referrals are offered a better discount than those without any.
This approach should be revisited, as there is little incentive for referred users to report their referrers honestly.
Post-Audit Conclusion (a8720219a6)
The Moby team provided us with an updated commit hash to evaluate the audit's remediations on.
During our re-evaluation of the codebase, we validated that the aforementioned Referral
related vulnerabilities have been alleviated.
In relation to the exhibits within the audit, we believe that the following exhibits have been misinterpreted and contain follow-up recommendations to aid in their consumption: CRE-04C
, SPD-02C
, FDR-02C
Additionally, the following exhibits have been partially addressed and should be addressed in full: RTR-03C
, VUS-03M
, VUS-01C
, VUS-05C
The Moby team additionally expressed their intentions to alleviate the following exhibits in our communications with them, however, we did not observe any alleviation applied: VUS-03C
, VUS-04C
, FPE-02S
, OMT-01S
, PVF-01S
, SPD-01S
, SPF-02S
, USD-02S
, ERG-01M
, ERR-01M
, ERS-01M
Post-Audit Conclusion (a95db4124c)
The Moby team supplied us with a follow up commit to address exhibits VUS-03M
, VUS-04C
, as well as RTR-03C
.
We confirmed that these exhibits have been properly alleviated in full, and corrected the alleviation chapters of certain exhibits that had been misconstrued; namely, exhibits VUS-03C
and VUS-05C
.
We consider all outputs of the audit report in relation to findings of minor severity and above to have been adequately consumed by the Moby team, and we do not anticipate any further remediative actions to be needed in relation to them.
Audit Synopsis
Severity | Identified | Alleviated | Partially Alleviated | Acknowledged |
---|---|---|---|---|
1 | 1 | 0 | 0 | |
138 | 108 | 0 | 30 | |
20 | 20 | 0 | 0 | |
1 | 1 | 0 | 0 | |
4 | 4 | 0 | 0 |
During the audit, we filtered and validated a total of 57 findings utilizing static analysis tools as well as identified a total of 107 findings during the manual review of the codebase. We strongly recommend that any minor severity or higher findings are dealt with promptly prior to the project's launch as they can introduce potential misbehaviours of the system as well as exploits.
Total Alleviations
The list below covers each segment of the audit in depth and links to the respective chapter of the report: