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

`Treasury`'s deposit accounting breaks with fee-on-transfer tokens

patitonar

Summary

The Treasury::deposit() function assumes the amount of tokens received equals the input amount parameter, which is incorrect for tokens that take fees on transfer or rebase balances.

Vulnerability Details

In the Treasury::deposit() function, the contract directly adds the input amount to its balance tracking:

function deposit(address token, uint256 amount) external override nonReentrant {
// ... existing code ...
IERC20(token).transferFrom(msg.sender, address(this), amount);
@> _balances[token] += amount;
@> _totalValue += amount;
// ... existing code ...
}

For tokens that:

  • Deduct fees on transfer (like STA, RFI)

  • Rebase balances (like AMPL)

The actual received amount will be less than the input amount. This creates a discrepancy between:

  • The contract's tracked balance (_balances[token])

  • The actual token balance (token.balanceOf(address(this)))

Impact

The mentioned discrepancy can lead to:

  • The _totalValue being incorrectly inflated

  • A revert when trying to withdraw tokens recorded in _balances due to insufficient balance

Tools Used

Manual review

Proof of Concept

Create a mock token that takes a 10% fee on transfers:

import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
​
contract ERC20MockFee is ERC20 {
constructor(string memory name, string memory symbol) ERC20(name, symbol) {}
​
function mint(address to, uint256 amount) external {
_mint(to, amount);
}
​
function _update(
address from,
address to,
uint256 amount
) internal virtual override {
if (from != address(0) && to != address(0)) {
// Extract 10% fee on transfers between non-zero addresses
uint256 fee = amount / 10;
uint256 finalAmount = amount - fee;
super._update(from, to, finalAmount);
super._update(from, address(this), fee);
} else {
// No fee on mints/burns
super._update(from, to, amount);
}
}
}

Add the following test to Treasury.test.js:

it("fee-on-transfer tokens", async () => {
// Deploy mock fee-on-transfer token
const MockFeeToken = await ethers.getContractFactory("ERC20MockFee");
const feeToken = await MockFeeToken.deploy("Fee Token", "FEE");
// Mint tokens to user1
const initialAmount = ethers.parseEther("1000");
await feeToken.mint(user1.address, initialAmount);
​
// Approve treasury to spend tokens
await feeToken.connect(user1).approve(treasury.getAddress(), initialAmount);
​
// Mock a 10% fee by transferring 90% of amount
const depositAmount = ethers.parseEther("100");
const feeAmount = depositAmount / 10n; // 10% fee
const actualReceived = depositAmount - feeAmount;
​
// Deposit into treasury
await treasury.connect(user1).deposit(feeToken.getAddress(), depositAmount);
​
// Treasury records full amount
expect(await treasury.getBalance(feeToken.getAddress())).to.equal(depositAmount);
expect(await treasury.getTotalValue()).to.equal(depositAmount);
​
// But actual balance is less due to fee
expect(await feeToken.balanceOf(treasury.getAddress())).to.equal(actualReceived);
​
// This causes issues when trying to withdraw the full tracked amount
await expect(
treasury.connect(manager).withdraw(feeToken.getAddress(), depositAmount, manager.address)
).to.be.revertedWithCustomError(feeToken, "ERC20InsufficientBalance");
});

Recommendations

Track actual received amounts by comparing balances before and after transfer

Updates

Lead Judging Commences

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

Treasury::deposit increments _totalValue regardless of the token, be it malicious, different decimals, FoT etc.

Treasury::deposit increments _balances[token] with amount, not taking FoT or rebasing into account

Support

FAQs

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