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

All collected fees in Treasury contract will be stuck because they are transferred without using `deposit` function.

tadev

Summary

Treasury contract is designed to manage protocol funds. The only contract that interacts with the Treasury contract is the FeeCollector contract, to transfer RAAC tokens in distributeCollectedFees and emergencyWithdraw functions.

The problem arises because both distributeCollectedFees and emergencyWithdraw functions send tokens to the Treasury contract using direct transfers with safeTransfer function.

Because deposit function is not used to send funds, _balances mapping token is not updated, making it impossible to execute future withdraw.

Vulnerability Details

The only way to withdraw tokens from Treasury contract is through the withdraw function:

function withdraw(address token, uint256 amount, address recipient)
external
override
nonReentrant
onlyRole(MANAGER_ROLE)
{
if (token == address(0)) revert InvalidAddress();
if (recipient == address(0)) revert InvalidRecipient();
if (_balances[token] < amount) revert InsufficientBalance();
​
_balances[token] -= amount;
_totalValue -= amount;
IERC20(token).transfer(recipient, amount);
​
emit Withdrawn(token, amount, recipient);
}

Withdrawals require that _balances[token] >= amount which will not be the case if deposit function is not used to send tokens.

Impact

The impact of this issue is high as it leads to protocol funds being stuck in the treasury forever.

Tools Used

Manual review.

Recommendations

Make sure to use deposit function of Treasury contract when distributing collected fees or during emergency withdraw procedure. Note that this will require an approval from the FeeCollector contract to the Treasury contract to allow transferFrom function to transfer tokens in deposit function. This approval can be done in FeeCollector constructor or with a decidated additionnal function.

Possible implementation for processDistribution and emergencyWithdraw:

function _processDistributions(uint256 totalFees, uint256[4] memory shares) internal {
uint256 contractBalance = raacToken.balanceOf(address(this));
if (contractBalance < totalFees) revert InsufficientBalance();
​
if (shares[0] > 0) {
uint256 totalVeRAACSupply = veRAACToken.getTotalVotingPower();
if (totalVeRAACSupply > 0) {
TimeWeightedAverage.createPeriod(
distributionPeriod, block.timestamp + 1, 7 days, shares[0], totalVeRAACSupply
);
totalDistributed += shares[0];
} else {
shares[3] += shares[0]; // Add to treasury if no veRAAC holders
}
}
​
if (shares[1] > 0) raacToken.burn(shares[1]);
if (shares[2] > 0) raacToken.safeTransfer(repairFund, shares[2]);
if (shares[3] > 0) ITreasury(treasury).deposit(address(raacToken), shares[3]);
}
function emergencyWithdraw(address token) external override whenPaused {
if (!hasRole(EMERGENCY_ROLE, msg.sender)) revert UnauthorizedCaller();
if (token == address(0)) revert InvalidAddress();
​
uint256 balance;
if (token == address(raacToken)) {
balance = raacToken.balanceOf(address(this));
ITreasury(treasury).deposit(address(raacToken), balance);
} else {
balance = IERC20(token).balanceOf(address(this));
ITreasury(treasury).deposit(token, balance);
}
​
emit EmergencyWithdrawal(token, balance);
}
Updates

Lead Judging Commences

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

FeeCollector::_processDistributions and emergencyWithdraw directly transfer funds to Treasury where they get permanently stuck

Support

FAQs

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