top of page

CRIT - Brahma.Fi - L2 Position handler miscalculates position value leading to severe risks

Target


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

  1. 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.

  2. 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.

  3. 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


Proof of Concept

Protocol insolvency scenario:

  1. Protocol currently has 9,000 USDC value

  2. User deposits 1,000 USDC, receives 1/10 total shares

  3. Protocol has 200 USDC yield, invests it in perpetual

  4. pertual investment backfires, positionvalue goes to -400

  5. pool total funds is calculated as 9,000 + 1000 + 400 = 10,400

  6. actual total funds: 9,000 + 1000 = 10,000

  7. User withdraws from pool, receives 1/10 of total value = 1,040 (minus fees)

  8. Therefore, user pulled out of vault +40 more than share is worth.



 

Brahma.Fi closed this submission saying that 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.

Recent Posts

See All

Comments


bottom of page