Description:
The `FeeCollector` contract have a function called `collectFees` that recive all the fees from the `msg.sender` and update the storage with the new fee amount, making this fees to be able to be shared and claimed. The problem is that the `RAACToken:_update` do not call this `FeeCollector:collectFees` function it transfer the tokens directly instead.
```solidity
function _update(
address from,
address to,
uint256 amount
) internal virtual override {
...
@> super._update(from, feeCollector, totalTax - burnAmount);
super._update(from, address(0), burnAmount);
super._update(from, to, amount - totalTax);
}
```
Impact:
These tokens tranfered will be blocked in the `feeCollector` contract forever
Proof of Concept:
<details><summary>Proof of Code</summary>
**Here are the steps to run the Foundry PoC:**
1. Open the `linux terminal`, `wsl` in windows.
2. `nomicfoundation` installation:
- If you have `npm` installed run this command:
- `npm install --save-dev @nomicfoundation/hardhat-foundry`
- If you have `yarn` installed run this command:
- `yarn add --dev @nomicfoundation/hardhat-foundry`
- If you have `pnpm` installed run this command:
- `pnpm add --save-dev @nomicfoundation/hardhat-foundry`
3. open the `hardhat.config.cjs`
- Paste this at the begining of the code:
- `require("@nomicfoundation/hardhat-foundry");`
4. run `npx hardhat init-foundry`
- This task will create a `foundry.toml` file with the right configuration and install `forge-std`
5. In the `test/` forlder create a new folder called `ProofOfCodes`
6. In this `test/ProofOfCodes` folder create a new file and paste the following code
7. To run the test you should run `forge test --mt test_feesAreWastedBecauseIsTransferingDirectly -vvvv`
```solidity
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
import {Test,console2} from "lib/forge-std/src/Test.sol";
import {DebtToken} from "contracts/core/tokens/DebtToken.sol";
import {RAACToken} from "contracts/core/tokens/RAACToken.sol";
import {DEToken} from "contracts/core/tokens/DEToken.sol";
import {RToken} from "contracts/core/tokens/RToken.sol";
import {veRAACToken} from "contracts/core/tokens/veRAACToken.sol";
import {FeeCollector, IFeeCollector} from "contracts/core/collectors/FeeCollector.sol";
import {Treasury} from "contracts/core/collectors/Treasury.sol";
contract StabilityPoolPoc is Test {
RAACToken raacToken;
FeeCollector feeCollector;
veRAACToken veRaacToken;
Treasury treasury;
address admin = makeAddr("admin");
address initialOwner = makeAddr("initialOwner");
address repairFund = makeAddr("repairFund");
address minter = makeAddr("minter");
address reciver = makeAddr("reciver");
address user = makeAddr("user");
function setUp() external {
vm.roll(block.number + 100);
vm.warp(block.timestamp + 10 days);
vm.startPrank(initialOwner);
treasury = new Treasury(admin);
raacToken = new RAACToken(initialOwner, 0, 0);
veRaacToken = new veRAACToken(address(raacToken));
feeCollector = new FeeCollector(address(raacToken), address(veRaacToken), address(treasury), repairFund, admin);
raacToken.setMinter(minter);
raacToken.setFeeCollector(address(feeCollector));
vm.stopPrank();
}
function test_feesAreWastedBecauseIsTransferingDirectly() external {
uint256 mintAmountWithoutFees = 10e18;
uint256 expectedFees = 150000000000000000;
vm.prank(minter);
raacToken.mint(reciver,mintAmountWithoutFees);
vm.prank(reciver);
raacToken.transfer(user, 10e18);
IFeeCollector.CollectedFees memory collectedFees = feeCollector.getCollectedFees();
uint256 totalAccumulatedFees = collectedFees.protocolFees + collectedFees.lendingFees + collectedFees.performanceFees + collectedFees.insuranceFees + collectedFees.mintRedeemFees + collectedFees.vaultFees + collectedFees.swapTaxes + collectedFees.nftRoyalties;
assert(raacToken.balanceOf(user) < 10e18); // check that fees were applied
assertEq(raacToken.balanceOf(user) + expectedFees, 10e18);
assert(totalAccumulatedFees != expectedFees);
assertEq(totalAccumulatedFees, 0); // check that the fee collector didn't stored the fees
}
}
```
</details>
Recommended Mitigation:
1. Call the `FeeCollector:collectRewards` instead of transfer it directly. (recommended)
```diff
function _update(
address from,
address to,
uint256 amount
) internal virtual override {
...
- super._update(from, feeCollector, totalTax - burnAmount);
+ FeeCollector(feeColletor).collectFees(totalTax - burnAmount, feeType);
super._update(from, address(0), burnAmount);
super._update(from, to, amount - totalTax);
}
```
2. Add a way to withdraw fees from the `FeeCollector`. (not recommended)