036.md

Rural Saffron Rooster

Medium

Logic error in allocate() function, prevents allocation of additional USUALS if new amount is lower than initial allocation

Summary

If, for example, Alice has an initial allocation of 1000 USUALS, and an attempt is made to allocate an additional 50 USUALS, the function allocate() will revert due to a logic error that prevents any new allocation that is lower than the previous allocation.

Root Cause

In allocate(), there is a logic error that blocks additional allocations of USUALS if the user already has an initial allocation that is greater than the new amount. For example, if two users (ALICE and BOB) have an initial allocation of 3.6 million USUALS, and a subsequent allocation of 3.5 million USUALS is attempted, the function will revert with a CannotReduceAllocation() error. As seen in the function at UsualSP.sol:350 , if the originalAllocation of the recipient is less than the previous allocation, the function will revert, blocking the new allocation. This effectively makes it impossible to provide a new allocation smaller than the initial amount unless removeOriginalAllocation() is called first, which would incur unnecessary gas costs, especially in the case of large array of users.

Internal pre-conditions

The user has an initial allocation of USUALS (e.g., 3.6 million for Alice and Bob). The new allocation attempt is for an amount less than the initial allocation (e.g., 3.5 million USUALS).

External pre-conditions

No response

Attack Path

No response

Impact

Users with an existing allocation of USUALS cannot receive a new allocation if the new amount is lower than their initial allocation. This restricts the flexibility of allocation and forces unnecessary calls to removeOriginalAllocation(), increasing gas costs. The problem is exacerbated when dealing with large arrays of users, making the additional gas fees significantly more costly.

PoC

    function test_newAllocation()public{
        setupStartOneDayRewardDistribution(2_000_000 ether);
        setupVestingWithOneYearCliff(3_600_000 ether); // 1% of USUALS_TOTAL_SUPPLY

        skip(VESTING_DURATION_THREE_YEARS);
        
        //Claim Alice
        vm.startPrank(alice);
        usualSP.claimReward();
        usualSP.claimOriginalAllocation();
        vm.stopPrank();
        
        //Claim Bob
        vm.startPrank(bob);
        usualSP.claimReward();
        usualSP.claimOriginalAllocation();
        vm.stopPrank();
        
        skip(ONE_MONTH);


        address[] memory path = new address[](2);
        path[0] = alice;
        path[1] = bob;


        uint256[] memory pathAllocation = new uint256[](2);
        pathAllocation[0] = 3_500_000 ether;
        pathAllocation[1] = 3_500_000 ether;



        uint256[] memory pathCliff = new uint256[](2);
        pathCliff[0] = 100;
        pathCliff[1] = 100;



        // New Allocation goes in revert because is lower that the previous one
        vm.startPrank(usualSPOperator);
        vm.expectRevert();
        usualSP.allocate(path, pathAllocation, pathCliff);

Mitigation

To address this issue, modify the logic in allocate() to allow additional allocations regardless of whether the new amount is less than the initial allocation. One potential fix could be to add the new allocation amount to the user’s existing balance rather than comparing it directly with the initial allocation amount.