Note: At the judges request here, this downgraded issue from the same warden has been included in this report for completeness.
https://github.com/code-423n4/2024-04-gondi/blob/b9863d73c08fcdd2337dc80a8b5e0917e18b036c/src/lib/pools/Pool.sol#L204
The base interest allocator is set in the Pool through two steps: setBaseInterestAllocator() and confirmBaseInterestAllocator(). The confirmBaseInterestAllocator() function allows anyone to call it.
This function will execute the logic using the _newBaseInterestAllocator input passed in by the caller, instead of the value set by the owner in setBaseInterestAllocator(). The verification for _newBaseInterestAllocator == getPendingBaseInterestAllocator is only performed when cachedAllocator != address(0).
As a result, if cachedAllocator == address(0), anyone can set an arbitrary address for the allocator. And since the asset is approved to the new allocator address, attacker could steal all the fund available in the Pool contract.
As we can see, the check if input _newBaseInterestAllocator matches the pending value is only performed when cachedAllocator != address(0). The same applies to the UPDATE_WAITING_TIME check.
If the contract is deployed without an allocator set up, or whenever the owner sets the allocator address to 0, an attacker could call confirmBaseInterestAllocator() to set it to an arbitrary address.
Consider running all the checks when cachedAllocator == address(0) as well.
Invalid Validation
0xA5DF (judge) commented:
0xend (Gondi) confirmed and commented:
0xA5DF (judge) decreased severity to Low and commented:
