Lyra_Audit_v02

Lyra Finance 19/01/23 Trust Security Smart Contract Audit Trust Security Lyra Finance Executive summary Findings Severity Total Fixed Acknowledged Disputed High 3 1 1 1 Medium 11 2 6 3 Low 5 1 3 1 Centralization score Centralized Decentralized Signature Category Options Audited file count 17 Lines of Code 5513 Auditor - Lead Trust Auditor - Junior 100proof Time period 09-19/01/23 3, High 11, Medium 5, Low FINDINGS Digitally signed by Trust Date: 2023-01-23 11:46:13 Trust Security Lyra Finance EXECUTIVE SUMMARY 1 DOCUMENT PROPERTIES 4 Versioning 4 Contact 4 INTRODUCTION 5 Scope 5 Repository details 5 About Trust Security 5 Disclaimer 5 Methodology 6 QUALITATIVE ANALYSIS 7 FINDINGS 8 High severity findings 8 TRST-H-1 canHedge may return wrong result when there is a pending position request 8 TRST-H-2 canHedge will return true when hedging requirement would be above the defined hedgeCap 8 TRST-H-3 misordered fee calculated causes collateral changes to be inaccurate 9 Medium severity findings 9 TRST-M-1 small LP providers may be unable to withdraw their deposits 9 TRST-M-2 ShortCollateral settleOptions may fail due to insolvency settled out of loop 10 TRST-M-3 base to quote swaps trust GMX-provided minPrice and maxPrice to be correct, which may be manipulated 11 TRST-M-4 sendAllFundsToLP() does not handle popular ERC20 tokens like BNB 11 TRST-M-5 recoverFunds() does not handle popular ERC20 tokens like BNB 12 TRST-M-6 option board will be settled with incorrect prices when settled after a delay 12 TRST-M-7 attackers can delay or disrupt hedging activity by abusing mutual exclusion with updateCollateral() 13 TRST-M-8 protectedQuote can be manipulated by calling processDepositQueue when large price moves in base asset occur 13 TRST-M-9 canHedge may return true when there is insufficient GMX liquidity to facilitate hedging, causing insolvency risks 14 TRST-M-10 canHedge will return true when trade moves delta in same direction as expectedHedge even when that leads to more risk 15 TRST-M-11 Decreasing a losing hedge position could make it overly-leveraged 16 Low severity findings 17 TRST-L-1 Attacker can take over GMXAdapter implementation contract 17 TRST-L-2 setPositionRouter leaks approval to previous positionRouter 17 Trust Security Lyra Finance TRST-L-3 PoolHedger can receive ETH directly from anyone 18 TRST-L-4 GMXAdapter minReturnPercent can be over 100%, requiring positive slippage for exchange to execute 18 TRST-L-5 Slippage in hedger may be double what is set in slippage parameter 19 Additional recommendations 20 Implement input validation when possible 20 Improve future-proofing by explicitly matching types 20 Misleading variable names 20 Redundant event parameters 20 Adopt single-validation pattern 20 Model-related risks 22 Centralization risks 22 LiquidityToken owner can freeze all token interactions and suspend the platform 22 Owner can manipulate board’s baseIv 23 Owner can drain liquidity pool instantly 23 Trust Security Lyra Finance Document properties Versioning Version Date Description 0.1 19/01/23 Client report 0.2 23/01/23 Team response Contact Or Cyngiser, AKA Trust boss@trustindistrust.com Trust Security Lyra Finance Introduction Trust Security has conducted an audit at the customer's request. The audit is focused on uncovering security issues and additional bugs contained in the code defined in scope. Some additional recommendations have also been given when appropriate. Given the proximity to launch date of the Newport upgrade, team has understandably decided to address only the more severe issues, while backlogging the rest of the findings. Scope ● BaseExchangeAdapter.sol ● GMXAdapter.sol ● GMXFuturesPoolHedger.sol ● LiquidityPool.sol ● LiquidityToken.sol ● OptionGreekCache.sol ● OptionMarket.sol ● OptionMarketPricer.sol ● OptionToken.sol ● ShortCollateral.sol ● BlackScholes.sol ● ConvertDecimals.sol ● FixedPointMathLib.sol ● GWAV.sol ● Math.sol ● PoolHedger.sol ● SimpleInitializable.sol Repository details ● Repository URL: https://github.com/lyra-finance/lyra-protocol ● Commit hash: cdcc2252f9a444e125b2725a8603fc3e70999371 About Trust Security Trust Security has been established by top-end blockchain security researcher Trust, in order to provide high quality auditing services. Trust is the leading auditor at competitive auditing service Code4rena, reported several critical issues to Immunefi bug bounty platform and is currently a Code4rena judge. Disclaimer Trust Security Lyra Finance Smart contracts are an experimental technology with many known and unknown risks. Trust Security assumes no responsibility for any misbehavior, bugs or exploits affecting the audited code or any part of the deployment phase. Furthermore, it is known to all parties that changes to the audited code, including fixes of issues highlighted in this report, may introduce new issues and require further auditing. Methodology In general, the primary methodology used is manual auditing. The entire in-scope code has been deeply looked at and considered from many different adversarial perspectives. Any additional dependencies on external code have also been reviewed. Specifically, close attention has been paid to changes since the recent Sherlock audit. Trust Security Lyra Finance Qualitative analysis Metric Rating Comments Code complexity Moderate Project has very high state complexity, but is well modularized. Documentation Good Project is mostly very well documented but could be more up to date. Best practices Excellent Code adheres to industry standards and best practices. Centralization risks Good The project has been overall designed to be as decentralized as possible. Trust Security Lyra Finance Findings High severity findings TRST-H-1 canHedge may return wrong result when there is a pending position request ● Category: Logical flaw ● Source: GMXFuturesPoolHedger.sol ● Status: Acknowledged Description Lyra’s security model relies on being able to hedge and achieve delta-neutrality when opening a user position. The check is done in canHedge() in GMXFuturesPoolHedger. The current hedge is calculated using _getCurrentHedgedNetDeltaWithSpot(). However, this function only takes the current position and ignores the pending increase/decrease position request. Therefore, canHedge result can be wrong - users may be rejected from interacting with the market, while attackers or innocent users may put the protocol in an unchangeable position. Recommended mitigation Including the pending position in the current hedge calculation. Team response While this is a valid issue, `canHedge` is an added safety rail rather than a critical component of the system. Unwanted option positions being opened to expose LPs to unwanted delta risk will cost attackers the fees to open option positions and all they achieve is exposing LPs to some limited directional/delta risk. Adding additional complexity to an already complex system feels unnecessary at this stage so this will not be implemented. TRST-H-2 canHedge will return true when hedging requirement would be above the defined hedgeCap ● Category: Logical flaw ● Source: GMXFuturesPoolHedger.sol ● Status: Disputed Description Lyra’s security model relies on being able to hedge and achieve delta-neutrality when opening a user position. The check is done in canHedge() in GMXFuturesPoolHedger. The expected hedge is fetched using _getCappedExpectedHedge(). However, it is never checked that the hedge has reached capacity, which sho