top of page

CRIT - Brahma.Fi - Fee collection does not take previous losses into account 🚩

Target


Bug Description

According to the docs (linked below) and as in any other investment scheme, the performance fee collected for user's earnings should be x% of profit, i.e. end_balance - start_balance.


Vault's collectFees() function collects fee at every deposit / withdraw event, by taking x% of the gains since last deposit/withdraw action. However, it does not account for losses in held funds, and therefore takes far too much fees from users. See scenario under POC section for example of loss for investor.


Impact

Permanent freezing of funds - user's funds are incorrectly consumed by the system as fees. Over time, since crypto markets are known as highly volatile, the majority of user funds will end up in the governance's hands.


Risk Breakdown

Likelyhood of exploit: Certain

Weakness: Incorrect handling of data

Severity: Critical


Recommendation

Two options:

  1. As done in other projects, store the maximum share price for which performance fee has been paid per user. Therefore, fluctuations in price will not continue to scoop unjust fees.

  2. In case of protocol losses, keep running total of (yield - losses) amount. Only charge performance fee if this delta is positive.


References


Proof of Concept

  1. Assume 15% performance fee as deployed

  2. User inserts 100$ to vault

  3. Vault loses 30$, now 70$

  4. User inserts additional 10$ to vault - prevVaultFunds updated to 80$

  5. Vault gains 30$ back, fee sent = 0.15 * 30 = 4.5$

  6. Vault now holds 105.5$

  7. User withdraws entire holdings, 105.5$.

  8. User permanently lost 4.5$ to incorrect performance fee collection.


Note that this applies to fees from both Vault and executor funds.


 

Immunefi has rejected the report, claiming that since bug relies on specific market movements and depends on when fee collection occurs. It is very disappointing that they did not understand that users will lose chunks of their deposits because markets never have a monotonous upward direction. They fail to differentiate between loss of yield and loss of original funds, and I still have no idea how this bug is not fixed.

Recent Posts

See All

Comments


bottom of page