Core Contracts

Summary

RAACReleaseOrchestrator::updateCategoryAllocation function doesn't check if the newAllowance parameter is zero. If 0 is supplied for a category, a non-zero value would not be able to be set as that category's allowance ever again.

Vulnerability Details

Consider a scenario when the RAACReleaseOrchestrator contract is created and no vesting schedule is created under a particular category. Thus categoryUsed[category] is 0 at this time.

Transaction 1: An account with admin role calls updateCategoryAllocation() with newAllocation as 0. This transaction succeeds and sets that category's allocation to 0.

Transaction 2: Later, if that category's allocation is planned to be set to some value > 0, the first line inside the function updateCategoryAllocation() will revert the transaction:

As there is no place where categoryUsed[category]'s value is decreased, the above risk does not exist for a category once vesting is initiated under that category... because of this check:

Impact

• Vesting RAACTokens using RAACReleaseOrchestrator for any account under a RAACReleaseOrchestrator::category would not be able to be initiated.

Likelihood: low

Impact: medium (another category's allocation can be increased to adjust for the lost allocation under one category)

Recommendations

Include a check that the newAllocation should not be zero:

Alternatively, if the protocol plans to have the flexibility of having zero allocation temporarily under a "valid" category, these steps can be taken in that case:

• Maintain a mapping of allowed categories: mapping(bytes32 => bool) public allowedCategories;

• At all the places in the contract where categoryAllocations[category] == 0 is used to check if the category is invalid, use the above mapping: if (!allowedCategory[category]) revert InvalidCategory();