Core Contracts

Summary

The Treasury contract's allocateFunds() function allows for overwriting existing allocations for a recipient without aggregation. This can lead to unintended loss or manipulation of allocated funds, disrupting expected fund distributions.

Vulnerability Details

The allocateFunds() function directly assigns the new allocation amount to the _allocations mapping, overwriting any previous allocation for the same allocator-recipient pair:

There is no mechanism for accumulating allocations or checking if a previous allocation exists. This behavior contradicts the potential expectation that allocateFunds() might be used to incrementally increase allocations over time.

Impact

This vulnerability can lead to the following issues:

• Loss of allocated funds: If the intention is to accumulate allocated funds, subsequent calls to allocateFunds() with different amounts will overwrite the previous allocation, effectively losing the initially allocated funds.

• Manipulation of fund distribution: An attacker with ALLOCATOR_ROLE can maliciously overwrite existing allocations, disrupting intended fund distributions and potentially denying funds to legitimate recipients.

• Accounting errors: Overwriting allocations without proper tracking or aggregation can lead to discrepancies in accounting and difficulty in reconciling allocated funds with actual disbursements.

Tools Used

• Manual Code Review: The vulnerability was identified through manual inspection of the allocateFunds() function.

• Fuzzing: Performed fuzzing to simulate different values for the allocation, and recipient address.

• Unit Testing: Unit tests are added to test the overwrite vulnerability.

Recommendations

1. Implement Allocation Aggregation: Modify the allocateFunds() function to aggregate new allocations with existing ones, instead of overwriting them. This can be achieved by adding the new amount to the existing allocation:

   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); // verify : event missing allocator address.

   }

2. Provide Allocation Adjustment Function: Consider adding a separate function for adjusting existing allocations, allowing allocators to increase or decrease the allocated amount. This provides more flexibility and control over allocations.

3. Add Allocation History: Consider storing an allocation history for each recipient, recording the time, amount, and allocator for each allocation event. This provides an audit trail and helps track allocation changes over time.

4. Implement Checks and Balances: Implement checks to ensure that total allocations do not exceed available funds in the treasury. This helps prevent over-allocation and potential insolvency.

5. Audit Contract: Conduct a formal security audit of the corrected code to identify and address any further potential vulnerabilities.

6. Test Thoroughly: Develop comprehensive unit and integration tests to verify the correctness of the allocateFunds() function and the allocation management logic under various scenarios, including multiple allocations, adjustments, and withdrawals.