Submission Details
Severity:
Overallocation is possible
Summary
The `allocateFunds()` function in `Treasury.sol` does not verify whether the allocation exceeds the treasury’s available funds. As a result, allocators can set an allocation amount that is higher than what is deposited, which may lead to discrepancies in fund management and potential operational issues.
Vulnerability Details
In the current implementation, the allocation mapping is updated directly with the provided amount without checking if sufficient funds are available:
```solidity
function allocateFunds(address recipient, uint256 amount) external override onlyRole(ALLOCATOR_ROLE) {
if (recipient == address(0)) revert InvalidRecipient();
if (amount == 0) revert InvalidAmount();
_allocations[msg.sender][recipient] = amount;
emit FundsAllocated(recipient, amount);
}
```
This means an allocator can allocate more funds than are present in the treasury.
For example, if the treasury holds 1,000 tokens, the allocator can still set an allocation of 1,500 tokens, resulting in an over-allocation by 500 tokens.
### Proof of Concept
The following unit test demonstrates the over-allocation issue:
```solidity
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
import "forge-std/Test.sol";
import "../../../contracts/core/collectors/Treasury.sol";
import "lib/openzeppelin-contracts/contracts/mocks/token/ERC20Mock.sol";
contract MockERC20 is ERC20 {
constructor(string memory name, string memory symbol) ERC20(name, symbol) {
_mint(msg.sender, 100_000 * 10 ** 18); // Mint 100k tokens to deployer
}
}
contract TreasuryTest is Test {
Treasury treasury;
MockERC20 token;
address admin = address(0x1);
address manager = address(0x2);
address allocator = address(0x3);
address user = address(0x7);
address userA = address(0x4);
address userB = address(0x6);
address recipient = address(0x5);
// Role hashes
bytes32 constant MANAGER_ROLE = keccak256("MANAGER_ROLE");
bytes32 constant ALLOCATOR_ROLE = keccak256("ALLOCATOR_ROLE");
function setUp() public {
// Deploy mock token
token = new MockERC20("Mock Token", "MTK");
// Deploy Treasury contract with admin
vm.prank(admin);
treasury = new Treasury(admin);
// Grant roles
vm.startPrank(admin);
treasury.grantRole(MANAGER_ROLE, manager);
treasury.grantRole(ALLOCATOR_ROLE, allocator);
vm.stopPrank();
// Transfer some tokens to user for testing deposits
token.transfer(user, 10_000 * 10 ** 18);
token.transfer(userA, 10_000 * 10 ** 18);
token.transfer(userB, 10_000 * 10 ** 18);
}
function testAllocateFundsAllowsOverAllocation() public {
// Deposit a limited amount of tokens
uint256 depositAmount = 1_000 * 10**18; // 1000 units available in treasury
vm.startPrank(user);
token.approve(address(treasury), depositAmount);
treasury.deposit(address(token), depositAmount);
vm.stopPrank();
// Verify treasury balance
assertEq(treasury.getBalance(address(token)), depositAmount, "Initial balance incorrect");
console.log("Treasury Balance:", depositAmount / 10**18, "units");
// Attempt to allocate more than available as allocator
uint256 overAllocationAmount = 1_500 * 10**18; // 1500 units, exceeds 1000
vm.prank(allocator);
treasury.allocateFunds(recipient, overAllocationAmount);
// Verify allocation succeeds despite insufficient funds
uint256 allocated = treasury.getAllocation(allocator, recipient);
assertEq(allocated, overAllocationAmount, "Allocation should succeed despite over-allocation");
console.log("Allocated Amount:", allocated / 10**18, "units");
// Highlight the issue: Allocation exceeds available balance
uint256 availableBalance = treasury.getBalance(address(token));
console.log("Available Balance:", availableBalance / 10**18, "units");
console.log("Over-Allocated by:", (allocated - availableBalance) / 10**18, "units");
// No check prevents this, so allocation > balance is possible
assertGt(allocated, availableBalance, "Allocation exceeds available funds, exposing the issue");
}
}
```
Test Output:
```yaml
Logs:
Treasury Balance: 1000 units
Allocated Amount: 1500 units
Available Balance: 1000 units
Over-Allocated by: 500 units
```
Impact
Over-Allocation Risk: Allocators can allocate funds beyond the actual treasury balance, causing a mismatch between recorded allocations and available funds.
Inaccurate Fund Management: Over-allocation may lead to operational issues, particularly during fund distribution or withdrawal processes, as the system assumes more funds are available than there actually are.
Potential Exploitation: This flaw could be exploited to manipulate fund distribution, potentially leading to mismanagement or loss of funds.
Tools Used
Manual Review and foundry
Recommendations
To prevent over-allocation, the contract should verify that the allocated amount does not exceed the treasury’s available funds. One potential solution is to include a check against the available balance before updating the allocation:
```solidity
function allocateFunds(address recipient, uint256 amount) external override onlyRole(ALLOCATOR_ROLE) {
if (recipient == address(0)) revert InvalidRecipient();
if (amount == 0) revert InvalidAmount();
// Retrieve the current available balance from the treasury
uint256 availableBalance = getBalance(address(token)); // Ensure correct token is referenced
require(amount <= availableBalance, "Allocation exceeds available funds");
_allocations[msg.sender][recipient] = amount;
emit FundsAllocated(recipient, amount);
}
```
This check would prevent allocators from over-allocating funds beyond what is available, ensuring accurate fund management and reducing potential exploitation.
Updates
Lead Judging Commences
inallhonesty 6 months ago
Submission Judgement Published Assigned finding tags:
Treasury::allocateFunds doesn't say what token you are actually allocating, doesn't check balances, or existing allocations to other recipients
Prize pool breakdown
Total prize
77,280 USDC
nSLOC
3,86420
USDC / LOC
74,000
3,280
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.