Description

The contract permits any user to deposit tokens via collectFee(), which updates the fee balances as soon as the deposit is made. When the fee distributor triggers distributeCollectedFees(), the contract calculates the total fees available and then distributes them among recipients. However, if new fee deposits occur between these operations, the calculated totals do not match, causing the function to revert. This can be exploited to create a denial-of-service (DoS) situation.

Vulnerability Detail

1.Fee Collection:
Any user can call collectFee(), and supply raacToken to the contract.

This internally updates the fee balance via _updateCollectedFees():

2.Fee Distribution Logic:
When distributeCollectedFees() is called, the process starts by reading the current fee balance. It then calculates how these fees should be distributed among the affected parties:

Here, _calculateTotalFees() aggregates the fee values that were previously recorded, and _calculateDistribution() does the breakdown to different recipients, summing up to totalCollected.

3.Race Condition and Its Exploit:

The problem arises because anyone can call collectFee() at any time. Imagine the following scenario:

• The distributor calls distributeCollectedFees() and the function reads totalFees from the current state.

• Before or as the internal loop in _calculateDistribution() runs, another user manages to call collectFee() and updates one of the fields in collectedFees.

• Now, when _calculateDistribution() runs, the sum totalCollected will reflect the updated fee amounts, which no longer match the totalFees that was computed at the very beginning of distributeCollectedFees().

• Because of the inequality between the locally computed totalCollected and the initial totalFees, the contract will hit the following check and revert:

This condition can occur in the following cases:

1. An unintentional user calls collectFee() during a distribution.

2. An attacker deliberately calls collectFee() during a distribution.

Attack path:

Users can simply burn their raacTokens via RAACToken::burn() function:

However, Alice may not find this interesting and decide to cause some trouble. She can call collectFee() with miniscule amounts of raacToken to update the fee balance every time she sees a dirtibution tx on the memepool. Even though she doesn't stand to gain anything, this interupts the normal flow within the protocol.

Impact

This check is supposed to act as a safeguard, ensuring that the fee distribution logic works on a consistent set of fee values and that they are not cleared out incorrectly. However, its reliance on a state that can be modified externally means that an attacker (or even an unintentional user) could deliberately call collectFee() during a distribution. Repeatedly doing so would cause this check to fail every time, effectively blocking fee distribution and potentially creating a denial-of-service (DoS) condition.

Tool used

Manual Review

Recommendation

Only distribute fees during contract pause.
This ensures that no new tokens can be deposited during the fee distribution process. This guarantees that the state used for the calculation remains unchanged, thereby ensuring that totalFees and totalCollected are consistent.