Summary
The Treasury::deposit and Treasury::withdraw functions in the Treasury contract do not properly account for fee-on-transfer tokens, such as RAAC Token. This leads to incorrect balance tracking, causing users to deposit more than what is actually received by the contract. As a result, withdrawals may fail due to insufficient funds, creating a denial-of-service (DoS) scenario where some users cannot retrieve their deposited tokens.
Vulnerability Details
Fee-on-transfer tokens deduct a portion of the transfer amount as a fee, meaning that the Treasury contract receives less than the deposited amount. However, the contract records the full deposit amount in its internal balances, leading to a mismatch between the recorded and actual token holdings. Consequently, the contract may allow withdrawals that exceed its actual token balance, resulting in failed transactions.
Affected Code in Treasury Contract
* @notice Deposits tokens into the treasury
* @dev Requires approval for token transfer
* @param token Address of token to deposit
* @param amount Amount of tokens to deposit
*/
function deposit(address token, uint256 amount) external override nonReentrant {
if (token == address(0)) revert InvalidAddress();
if (amount == 0) revert InvalidAmount();
@> IERC20(token).transferFrom(msg.sender, address(this), amount);
@> _balances[token] += amount;
_totalValue += amount;
emit Deposited(token, amount);
}
Steps to Reproduce
Two users deposit 100e18 RAAC tokens each into the Treasury.
Due to the transfer fee, the actual amount received by the Treasury is less than 200e18 RAAC.
The recorded _balances[token] still reflects 200e18 RAAC.
When attempting to withdraw, the first user successfully withdraws 100e18 RAAC.
The second user's withdrawal fails due to insufficient balance in the Treasury.
A simple proof-of-concept is as follows:
pragma solidity 0.8.20;
import {Test, console} from "forge-std/Test.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "src/core/tokens/RAACToken.sol";
import "src/core/collectors/Treasury.sol";
contract TreasuryDenialOfServiceTest is Test {
Treasury internal treasury;
RAACToken internal raac;
address internal admin;
address internal minter;
address internal user;
function setUp() public {
admin = address(this);
minter = makeAddr("minter");
user = makeAddr("user");
treasury = new Treasury(admin);
raac = new RAACToken(admin, 0, 0);
raac.setMinter(minter);
vm.prank(minter);
raac.mint(minter, 100e18);
vm.prank(minter);
raac.mint(user, 100e18);
}
function testExploit() public {
vm.startPrank(user);
raac.approve(address(treasury), 100e18);
treasury.deposit(address(raac), 100e18);
vm.stopPrank();
vm.startPrank(minter);
raac.approve(address(treasury), 100e18);
treasury.deposit(address(raac), 100e18);
vm.stopPrank();
assertNotEq(raac.balanceOf(address(treasury)), treasury.getBalance(address(treasury)));
treasury.withdraw(address(raac), 100e18, user);
vm.expectRevert();
treasury.withdraw(address(raac), 100e18, minter);
}
}
Impact
Denial of Service (DoS): Users may not be able to withdraw their deposited tokens.
Incorrect Accounting: The contract overestimates its token holdings, leading to financial mismanagement.
Potential Fund Loss: If withdrawals continue to fail, some users may permanently lose access to their funds.
Tools Used
Manual Review
Recommendations
Properly Track Fee-on-Transfer Tokens
Modify the deposit function to update balances based on the actual amount received:
function deposit(address token, uint256 amount) external override nonReentrant {
if (token == address(0)) revert InvalidAddress();
if (amount == 0) revert InvalidAmount();
+ uint256 balanceBefore = IERC20(token).balanceOf(address(this));
IERC20(token).transferFrom(msg.sender, address(this), amount);
+ uint256 balanceAfter = IERC20(token).balanceOf(address(this));
+ uint256 actualReceived = balanceAfter - balanceBefore;
- _balances[token] += amount;
- _totalValue += amount;
+ _balances[token] += actualReceived;
+ _totalValue += actualReceived;
emit Deposited(token, actualReceived);
}