Vertex audit
A private security review of Vertex (hybrid orderbook DEX), conducted with Three Sigma and completed on 3rd April 2024 over 40 days. 0xSimao disclosed 24 findings: 4 high severity, 1 medium, 16 low and 3 informational.
What Vertex is
Vertex is a decentralised exchange built on a hybrid orderbook and AMM model: spot and perpetual markets, lending and borrowing inside one cross-margined clearinghouse, chasing the speed of a centralised venue with on-chain settlement.
Scope
A fourteen person-week Three Sigma engagement across the core exchange: the clearinghouse, its spot and perpetual engines, the endpoint sequencing layer, margin and liquidation logic, and the ArbAirdrop distribution contract.
High severity findings
- ClearingHouseLiq::_assertLiquidationAmount() may increase basis points due to negative quoteBalance.amount + insurance
- state.cumulativeDepositsMultiplierX18 may become 0 or negative, leading to loss of funds
- OffchainExchange::swapAmm() does not validate that txn.priceX18 > 0, allowing donation attacks
- ClearingHouseLiq::_finalizeSubaccount() does not check if the subaccount has lps
Medium severity findings
Low severity findings
- Verifier::checkQuorum() returns false with more than 3 signers
- Downcasting Leads to Silent Overflow
- isHealthy() Function Always Returns True
- Precision loss due to engines not using .muldiv() when calculating lp ratios
- Unused argument in Endpoint::registerTransferableWallet()
- Risk parameters should include additional checks to prevent mistakes
- PerpEngineLp::burnLp() and SpotEngineLp::burnLp() are missing slippage checks
- ClearingHouse::addEngine() initializes productEngine, so it may be frontrunned and someone initializes productEngine instead
- Any user may enforce the referral code of other new users
- txType == TransactionType.ExecuteSlowMode may cause batch of transactions to fail due to _slowModeConfig.txUpTo < _slowModeConfig.txCount
- User deposits are exposed to protocol risk before being able to do anything
- Ratios in PerpEngineLp or SpotEngineLp may be manipulated when the liquidity is low
- Reevaluate the Need for renounceOwnership()
- Missing pagination for some functions that iterate
- Ownable2Step is preferred over Ownable
- Missing events
Informational findings
- IERC20Base Noncompliant With ERC20 Standard
- Use of abi.encodePacked() with Dynamic Types
- RiskHelper::isoGroup() returns 0, regardless of the passed subaccount
The report