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

Multiple Token Management Lets Withdraw a Token Different than Deposited Token

TxGuard

Description

The Treasury contract allows for the management of multiple ERC20 tokens. However, the current implementation does not adequately differentiate between tokens during allocation and withdrawal processes. This can lead to significant risks, including mismanagement of funds and confusion regarding which tokens are allocated to which recipients.

Key Issues

  1. Lack of Token-Specific Tracking:

    • The contract does not maintain separate records for allocations based on the token type. This means that allocators can allocate funds without regard to the specific token being used, leading to potential confusion and mismanagement.

  2. Withdrawals Not Token-Specific:

    • Managers can withdraw any token from the treasury without considering the allocations made for that specific token. This can result in situations where funds are withdrawn that were promised to recipients in a different token.

  3. Potential for Financial Mismanagement:

    • If multiple tokens are managed without clear tracking, it can lead to scenarios where the treasury may not have enough of a specific token to fulfill its obligations, even if the total balance across all tokens appears sufficient.

Example Scenario

  1. Initial Setup:

    • The treasury holds 1,000 DAI (a stablecoin) and 1,000 USDC (another stablecoin).

    • Allocator A allocates 500 DAI to Recipient 1.

    • Allocator B allocates 500 USDC to Recipient 2.

  2. Withdrawals Made:

    • The manager decides to withdraw 1,000 USDC from the treasury.

  3. Problem Arises:

    • The treasury has enough total funds (1,000 DAI + 1,000 USDC), but the withdrawal of 1,000 USDC does not consider the allocations made for DAI.

    • If the treasury had promised 500 DAI to Recipient 1, the withdrawal could lead to a situation where the treasury cannot fulfill its obligations to Recipient 1.

Consequences

  • Unmet Obligations: Recipients may not receive the funds they were promised, leading to frustration and loss of trust in the protocol.

  • Financial Instability: The treasury may face financial instability if it cannot meet its obligations, potentially leading to legal issues or the need for emergency measures.

  • Reputation Damage: The protocol's reputation may suffer if users perceive it as unreliable or poorly managed.

POC:

forge test --match-test testMultipleTokenDepositsAndWithdrawal -vv

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import {Test} from "forge-std/Test.sol";
import {console} from "forge-std/console.sol";
import {Treasury} from "../contracts/core/collectors/Treasury.sol";
import {MockERC20} from "../test/mocks/MockERC20.sol";
​
contract TreasuryTest is Test {
Treasury public treasury;
MockERC20 public token;
MockERC20 public token2;
​
address admin = makeAddr("admin");
address manager = makeAddr("manager");
address allocator = makeAddr("allocator");
address allocator2 = makeAddr("allocator2");
address user = makeAddr("user");
address recipient = makeAddr("recipient");
​
uint256 constant INITIAL_BALANCE = 1000e18;
​
event Deposited(address indexed token, uint256 amount);
event Withdrawn(address indexed token, uint256 amount, address indexed recipient);
event FundsAllocated(address indexed recipient, uint256 amount);
​
function setUp() public {
console.log("==== Setting Up Test Environment ====");
// Deploy contracts
treasury = new Treasury(admin);
token = new MockERC20("Test Token", "TEST");
token2 = new MockERC20("Test Token 2", "TEST2");
token2.setDecimals(6);
​
console.log("Deployed Treasury & Token contracts");
console.log("Admin:", admin);
console.log("Manager:", manager);
console.log("Allocator1:", allocator);
console.log("Allocator2:", allocator2);
console.log("User:", user);
console.log("Recipient:", recipient);
​
// Setup roles
vm.startPrank(admin);
treasury.grantRole(treasury.MANAGER_ROLE(), manager);
treasury.grantRole(treasury.ALLOCATOR_ROLE(), allocator);
treasury.grantRole(treasury.ALLOCATOR_ROLE(), allocator2);
vm.stopPrank();
console.log("Roles granted successfully");
​
// Setup initial token balances
token.mint(user, INITIAL_BALANCE);
token2.mint(user, 1000e6);
console.log("Initial token balances minted for user");
}
​
function testMultipleTokenDepositsAndWithdrawal() public {
// Define variables for token amounts
uint256 TOKEN1_AMOUNT = INITIAL_BALANCE;
uint256 TOKEN2_AMOUNT = 1000e6;
​
console.log("==== Starting Multiple Token Deposits and Withdrawal Test ====");
​
// User deposits tokens into the treasury
vm.startPrank(user);
console.log("User is approving deposits for Token1 and Token2");
token.approve(address(treasury), TOKEN1_AMOUNT);
token2.approve(address(treasury), TOKEN2_AMOUNT);
​
// Log user token balances before deposit
uint256 balanceTokenBefore = token.balanceOf(user);
uint256 balanceToken2Before = token2.balanceOf(user);
console.log("User Token1 balance before deposit:", balanceTokenBefore);
console.log("User Token2 balance before deposit:", balanceToken2Before);
​
// Deposit tokens into treasury
treasury.deposit(address(token), TOKEN1_AMOUNT);
console.log("Deposited Token1, amount:", TOKEN1_AMOUNT);
treasury.deposit(address(token2), TOKEN2_AMOUNT);
console.log("Deposited Token2, amount:", TOKEN2_AMOUNT);
​
// Log user token balances after deposit
uint256 balanceTokenAfter = token.balanceOf(user);
uint256 balanceToken2After = token2.balanceOf(user);
console.log("User Token1 balance after deposit:", balanceTokenAfter);
console.log("User Token2 balance after deposit:", balanceToken2After);
vm.stopPrank();
console.log("User deposit actions completed");
​
// Allocator allocates funds (implicitly linked to Token1 deposit)
vm.startPrank(allocator);
console.log("Allocator is allocating funds for recipient");
treasury.allocateFunds(recipient, TOKEN1_AMOUNT);
vm.stopPrank();
console.log("Funds allocated successfully for recipient");
​
// Manager withdraws funds from Token2 (mismatching the allocated token)
vm.startPrank(manager);
console.log("Manager is withdrawing Token2");
treasury.withdraw(address(token2), TOKEN2_AMOUNT, manager);
vm.stopPrank();
console.log("Manager withdrawal completed for Token2");
​
// Final logging: Show treasury's total value to highlight the issue
uint256 treasuryTotal = treasury.getTotalValue();
console.log("Final Treasury Total Value:", treasuryTotal);
console.log("Notice: Allocation was made for Token1, but withdrawal occurred from Token2.");
console.log("This discrepancy might indicate issues due to mixing token decimals and misallocated withdrawals.");
}
}
​

Recommendations

  1. Implement Token-Specific Tracking:

    • Maintain separate records for allocations based on the token type to ensure clarity and prevent mismanagement.

  2. Token-Specific Withdrawals:

    • Modify the withdrawal function to ensure that withdrawals are only allowed for tokens that have been allocated and that the total amount withdrawn does not exceed the available balance for that specific token.

  3. Enhanced Documentation:

    • Clearly document the handling of multiple tokens in the contract to prevent misunderstandings among users and developers.Multiple Token Management Lets Withdraw a Token Different than Deposited Token

Updates

Lead Judging Commences

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

Treasury::allocateFunds doesn't say what token you are actually allocating, doesn't check balances, or existing allocations to other recipients

Support

FAQs

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