Core Contracts

Summary

The allocateFunds() function in the Treasury.sol contract does not correctly accumulate fund allocations. Instead of adding the new allocation amount to the existing value, it resets the allocation for the recipient to the new amount. This behavior leads to unintended overwriting of previous allocations, which may cause incorrect fund tracking and misallocation of treasury funds.

Vulnerability Details

Issue: Allocation Amount is Overwritten Instead of Incremented

Code Snippet from Treasury.sol

Expected Behavior:

When an allocator assigns funds to a recipient, the allocated amount should be added to any existing allocation.

Actual Behavior:

The function directly assigns amount to _allocations[msg.sender][recipient], completely replacing any previous allocation instead of incrementing it.

Problems Caused by This Issue:

1. Loss of Previous Allocations: Each time allocateFunds() is called for a recipient, the previous allocation amount is erased, leading to incorrect allocation tracking.

2. Misallocation of Treasury Funds: Allocators may unknowingly override previously allocated funds, leading to improper fund distribution.

Impact

• Inconsistent Fund Tracking: Allocations do not accumulate as expected, leading to inaccurate records.

• Potential Loss of Allocated Funds: If an allocator intends to distribute funds progressively, prior allocations are lost with each new transaction.

Tools Used

• Manual Code Review

Recommendations

1. Modify allocateFunds() to Accumulate Allocations Instead of Overwriting:

   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; // Accumulate instead of overwrite

   emit FundsAllocated(recipient, _allocations[msg.sender][recipient]);

   }

   ​

2. Add Event Logging for Previous and New Allocation Amounts:

   emit FundsAllocated(recipient, previousAmount, _allocations[msg.sender][recipient]);

   ​