Core Contracts
Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

FeeCollector cannot burn all the RAAC tokens that meant to be burned when distributes collected fees

h2134

Summary

FeeCollector cannot burn all the RAAC tokens that meant to be burned when distributes collected fees.

Vulnerability Details

When distributes collected fees, some fees are expected to burned if the corresponding fee type's burnShare is not 0.

FeeCollector::_processDistributions()

if (shares[1] > 0) raacToken.burn(shares[1]);

However, when burn() is called in RAACToken, burn tax is charged and if feeCollector is not address(0), some RAAC tokens are sent back to FeeCollector, results in some tokens cannot be fully burned.

RAACToken::burn()

function burn(uint256 amount) external {
uint256 taxAmount = amount.percentMul(burnTaxRate);
_burn(msg.sender, amount - taxAmount);
if (taxAmount > 0 && feeCollector != address(0)) {
_transfer(msg.sender, feeCollector, taxAmount);
}
}

Impact

burnRate is set in FeeCollector to reduce the total circulating supply, creating a scarcity of the cryptocurrency. Fail to burn the expected amount of RAAC tokens will not increase the token's value based on basic supply and demand dynamics.

POC

Please run forge test --mt testAudit_CannotFullyBurn.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import {Test, console, stdError} from "forge-std/Test.sol";
import "@openzeppelin/contracts/utils/math/SafeCast.sol";
import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
​
import "../contracts/libraries/math/WadRayMath.sol";
import "../contracts/core/pools/LendingPool/LendingPool.sol";
import "../contracts/core/pools/StabilityPool/StabilityPool.sol";
import "../contracts/mocks/core/tokens/crvUSDToken.sol";
import "../contracts/core/tokens/RToken.sol";
import "../contracts/core/tokens/DebtToken.sol";
import "../contracts/core/tokens/DeToken.sol";
import "../contracts/core/tokens/RAACToken.sol";
import "../contracts/core/tokens/RAACNFT.sol";
import "../contracts/core/tokens/veRAACToken.sol";
import "../contracts/core/primitives/RAACHousePrices.sol";
import "../contracts/core/minters/RAACMinter/RAACMinter.sol";
import "../contracts/core/collectors/FeeCollector.sol";
import "../contracts/core/collectors/Treasury.sol";
​
contract Audit is Test {
using WadRayMath for uint256;
using SafeCast for uint256;
​
address owner = makeAddr("Owner");
address repairFund = makeAddr("RepairFund");
​
LendingPool lendingPool;
StabilityPool stabilityPool;
RAACHousePrices raacHousePrices;
​
crvUSDToken crvUSD;
RToken rToken;
DebtToken debtToken;
DEToken deToken;
RAACToken raacToken;
RAACNFT raacNft;
veRAACToken veRaacToken;
​
RAACMinter raacMinter;
FeeCollector feeCollector;
Treasury treasury;
​
function setUp() public {
vm.warp(1 days);
​
raacHousePrices = new RAACHousePrices(owner);
​
// Deploy tokens
raacToken = new RAACToken(owner, 100, 50);
veRaacToken = new veRAACToken(address(raacToken));
crvUSD = new crvUSDToken(owner);
rToken = new RToken("RToken", "RToken", owner, address(crvUSD));
debtToken = new DebtToken("DebtToken", "DT", owner);
raacNft = new RAACNFT(address(crvUSD), address(raacHousePrices), owner);
deToken = new DEToken("DEToken", "DEToken", owner, address(rToken));
​
// Deploy Treasury and FeeCollector
treasury = new Treasury(owner);
​
feeCollector = new FeeCollector(
address(raacToken),
address(veRaacToken),
address(treasury),
repairFund,
owner
);
​
// Deploy LendingPool
lendingPool = new LendingPool(
address(crvUSD),
address(rToken),
address(debtToken),
address(raacNft),
address(raacHousePrices),
0.1e27
);
lendingPool.transferOwnership(owner);
​
// Deploy stabilityPool Proxy
bytes memory data = abi.encodeWithSelector(
StabilityPool.initialize.selector,
address(rToken),
address(deToken),
address(raacToken),
address(owner),
address(crvUSD),
address(lendingPool)
);
address stabilityPoolProxy = address(
new TransparentUpgradeableProxy(
address(new StabilityPool(owner)),
owner,
data
)
);
stabilityPool = StabilityPool(stabilityPoolProxy);
​
// RAACMinter
raacMinter = new RAACMinter(
address(raacToken),
address(stabilityPool),
address(lendingPool),
owner
);
​
// Initialization
vm.startPrank(owner);
raacHousePrices.setOracle(owner);
​
rToken.setReservePool(address(lendingPool));
​
debtToken.setReservePool(address(lendingPool));
​
deToken.setStabilityPool(address(stabilityPool));
​
stabilityPool.setRAACMinter(address(raacMinter));
​
lendingPool.setStabilityPool(address(stabilityPool));
​
raacToken.setMinter(address(raacMinter));
raacToken.setFeeCollector(address(feeCollector));
raacToken.manageWhitelist(address(feeCollector), true);
raacToken.manageWhitelist(address(raacMinter), true);
raacToken.manageWhitelist(address(stabilityPool), true);
​
vm.stopPrank();
​
vm.label(address(crvUSD), "crvUSD");
vm.label(address(rToken), "RToken");
vm.label(address(debtToken), "DebtToken");
vm.label(address(deToken), "DEToken");
vm.label(address(raacToken), "RAACToken");
vm.label(address(raacNft), "RAAC NFT");
vm.label(address(lendingPool), "LendingPool");
vm.label(address(stabilityPool), "StabilityPool");
vm.label(address(raacMinter), "RAACMinter");
vm.label(address(veRaacToken), "veRAAC");
vm.label(address(feeCollector), "FeeCollector");
vm.label(address(treasury), "Treasury");
vm.label(repairFund, "RepairFund");
}
​
function testAudit_CannotFullyBurn() public {
uint256 distributeAmount = 10000e18;
address feeDistributor = makeAddr("FeeDistributor");
deal(address(raacToken), feeDistributor, distributeAmount);
​
vm.startPrank(owner);
feeCollector.grantRole(feeCollector.DISTRIBUTOR_ROLE(), feeDistributor);
raacToken.manageWhitelist(feeDistributor, true);
vm.stopPrank();
​
vm.startPrank(feeDistributor);
raacToken.approve(address(feeCollector), distributeAmount);
feeCollector.collectFee(distributeAmount, 6);
feeCollector.distributeCollectedFees();
vm.stopPrank();
​
// Tokens expected to be burned are transferred back to FeeCollector
assertEq(raacToken.balanceOf(address(feeCollector)), 2.5e18);
}
}
​

Tools Used

Manual Review

Recommendations

It is recommended not to charge burn tax when the caller is FeeCollector.

function burn(uint256 amount) external {
+ if (msg.sender == feeCollector) {
+ _burn(msg.sender, amount);
+ return;
+ }
uint256 taxAmount = amount.percentMul(burnTaxRate);
_burn(msg.sender, amount - taxAmount);
if (taxAmount > 0 && feeCollector != address(0)) {
_transfer(msg.sender, feeCollector, taxAmount);
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

RAACToken::burn applies burn tax twice when transferring to feeCollector, causing excess tokens to be burned and reduced fees to be collected

This is by design, sponsor's words: Yes, burnt amount, done by whitelisted contract or not always occur the tax. The feeCollector is intended to always be whitelisted and the address(0) is included in the _transfer as a bypass of the tax amount, so upon burn->_burn->_update it would have not applied (and would also do another burn...). For this reason, to always apply such tax, the burn function include the calculation (the 2 lines that applies) and a direct transfer to feeCollector a little bit later. This is done purposefully

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

RAACToken::burn applies burn tax twice when transferring to feeCollector, causing excess tokens to be burned and reduced fees to be collected

This is by design, sponsor's words: Yes, burnt amount, done by whitelisted contract or not always occur the tax. The feeCollector is intended to always be whitelisted and the address(0) is included in the _transfer as a bypass of the tax amount, so upon burn->_burn->_update it would have not applied (and would also do another burn...). For this reason, to always apply such tax, the burn function include the calculation (the 2 lines that applies) and a direct transfer to feeCollector a little bit later. This is done purposefully

Appeal created

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

RAACToken::burn applies burn tax twice when transferring to feeCollector, causing excess tokens to be burned and reduced fees to be collected

This is by design, sponsor's words: Yes, burnt amount, done by whitelisted contract or not always occur the tax. The feeCollector is intended to always be whitelisted and the address(0) is included in the _transfer as a bypass of the tax amount, so upon burn->_burn->_update it would have not applied (and would also do another burn...). For this reason, to always apply such tax, the burn function include the calculation (the 2 lines that applies) and a direct transfer to feeCollector a little bit later. This is done purposefully

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!