Core Contracts

Summary

The distributeCollectedFees function in the FeeCollector contract incorrectly sends shares[3] (the treasury share) to the treasury using safeTransfer. This approach does not update the treasury's internal balance mapping or the total value variable, leading to issues when users with the MANAGER_ROLE attempt to withdraw tokens from the treasury.

Vulnerability Details

The issue arises from the way the treasury receives its allocated tokens. Instead of depositing the tokens into the treasury contract with the function deposit(), the safeTransfer method is used, which bypasses the treasury's internal accounting mechanisms.

Example Scenario:

1. Token Distribution: When the distributeCollectedFees function is called, the shares for the treasury are calculated and sent using:

   if (shares[3] > 0) raacToken.safeTransfer(treasury, shares[3]);

2. Lack of Internal Accounting: The safeTransfer method directly transfers the tokens to the treasury contract but does not update the treasury's mapping _balances or the _totalValue variable. As a result, the treasury contract does not recognize that it has received these tokens.

3. Withdrawal Attempt: When a user with the MANAGER_ROLE tries to withdraw these tokens from the treasury, the withdrawal function checks the internal balance mapping to determine how many tokens are available for withdrawal. Since the tokens were not deposited correctly, the mapping does not reflect the presence of these tokens.

4. Resulting Issue: The manager is unable to withdraw any tokens because the treasury's internal state does not recognize the tokens that were sent via safeTransfer. This effectively locks the tokens in the treasury contract, making them inaccessible.

Impact

This vulnerability leads to a situation where tokens are effectively "stuck" in the treasury contract, as they cannot be withdrawn by users with the appropriate role. This undermines the intended functionality of the treasury.

Tools Used

• Manual code review

Recommendations

Use Deposit Method: Modify the token transfer logic to use the deposit() function instead of safeTransfer. This will ensure that the treasury's internal balance mapping and total value variable are updated correctly when tokens are received.