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

Issue with Token Transfers and Internal Balance Update in Treasury Contract

denisturgenev2021

Summary

This report identifies a critical issue in the interaction between the FeeCollector contract and the Treasury contract. The problem arises from the way tokens are transferred to the Treasury using the safeTransfer method without updating the internal balance tracking state variable (_balances[token]) in the Treasury contract. This can lead to a situation where the contract’s internal state does not reflect the actual token balance, preventing the successful withdrawal of funds from the treasury.

Vulnerability Details

In the current implementation, tokens are sent from the FeeCollector contract to the Treasury contract using the following line of code:

raacToken.safeTransfer(treasury, shares[3]);
raacToken.safeTransfer(treasury, balance);

However, the Treasury contract maintains an internal balance (_balances[token]) which is only updated through the deposit function. Since the safeTransfer method is directly used to send tokens, the internal state does not get updated with the new token balance. As a result, the withdraw() function in the Treasury contract, which checks the balance of tokens using _balances[token], will return incorrect values and fail to allow withdrawals, even though the contract holds the correct amount of tokens.

Impact

The issue can have several severe consequences:

  • Token Depletion Risk: Since internal balances are not updated when tokens are transferred, the treasury contract will report an incorrect token balance. This can lead to failed withdrawals or incorrect accounting of available funds.

  • Security Breach: Attackers or malicious actors may exploit this behavior to make it appear that tokens are not available in the treasury, which could lead to the loss of funds or manipulation of token distribution processes.

  • User Confusion: Users and auditors may be unable to determine the true state of the treasury, leading to confusion regarding available assets.

Tools Used

Manual code review

Recommendations

To resolve this vulnerability and ensure the correct functionality of the treasury contract, the following changes should be made:

In the FeeCollector contract, after transferring tokens to the Treasury, ensure that the deposit() function is called to update the internal balance:

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));
- raacToken.safeTransfer(treasury, balance);
+ ITreasury(treasury).deposit(address(raacToken), balance);
} else {
balance = IERC20(token).balanceOf(address(this));
SafeERC20.safeTransfer(IERC20(token), treasury, balance);
}
​
emit EmergencyWithdrawal(token, balance);
}
​

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) raacToken.safeTransfer(treasury, shares[3]);
+ if (shares[3] > 0) ITreasury(treasury).deposit(address(raacToken), shares[3]);
}
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.