Brahma

--> CRIT - Brahma.Fi - L2 Position handler miscalculates position value leading to severe risks top of page Search Target https://optimistic.etherscan.io/address/0x1b6BF7Ab4163f9a7C1D4eCB36299525048083B5e Description Bug Description In PerpV2Controller, positionInWantToken() view calculates value held in Perp account, which is updated by keeper on L1 using PerpTradeExecutor's setPosValue. Value is calculated using the following block: int256 posValue = clearingHouse.getAccountValue(address(this)); uint256 amountOut = (posValue < 0) ? uint256(-1 * posValue) : uint256(posValue); The problem is confusion regarding if accountValue is negative. It does not mean value in short position, which is the case in accountBalance.getTotalPositionValue(), but actually that the account is underwater. Therefore, when the high-risk weekly Perp strategy goes negative, it will show up as positive funds! This has several severe consequences detailed under impacts section. Impact When batcher deposits user's funds, it will return a smaller amount of shares than should be, therefore users immediately lose value after negative position is settled. When batcher withdraws user's funds, it will return too many tokens to the user, thinking there are more funds than in reality. Therefore, protocol solvency is at risk. In fee collection, system will collect too much fees because totalExecutorFunds() is miscalculated. If the system supports non-batch mode, users can directly exploit this by withdrawing their funds immediately. Risk Breakdown Difficulty to Exploit: Easy Weakness: Sign confusion Likelihood: High Recommendation In positionInWantTokens(), is posValue < 0, the holding is worthless, return 0. References Perpetual code samples ( https://support.perp.com/hc/en-us/articles/7917807368729-Perp-v2-Integration-Guide-Code-Samples ) Perpetual source code ( https://github.com/perpetual-protocol/perp-curie-contract ) Proof of Concept Protocol insolvency scenario: Protocol currently has 9,000 USDC value User deposits 1,000 USDC, receives 1/10 total shares Protocol has 200 USDC yield, invests it in perpetual pertual investment backfires, positionvalue goes to -400 pool total funds is calculated as 9,000 + 1000 + 400 = 10,400 actual total funds: 9,000 + 1000 = 10,000 User withdraws from pool, receives 1/10 of total value = 1,040 (minus fees) Therefore, user pulled out of vault +40 more than share is worth. Brahma.Fi closed this submission saying t hat accountValue can never be negative as the position will be liquidated at margin value of 6.25%. This statement is ridiculous, since they are counting on external liquidator bots to decide it's time to liquidate, in order for the protocol to remain solvent. The sign confusion is clearly unintentional and dangerous, but they had to come up with some excuse not to mark the report as valid. bottom of page