confirmBaseInterestAllocator() change BaseInterestAllocator may pay large getReallocationBonus

[c4-bot-7]

Lines of code

https://github.com/code-423n4/2024-04-gondi/blob/b9863d73c08fcdd2337dc80a8b5e0917e18b036c/src/lib/pools/Pool.sol#L211

Vulnerability details

Vulnerability details

owner can submit getPendingBaseInterestAllocator first, and then anyone can enable it by confirmBaseInterestAllocator().

    function confirmBaseInterestAllocator(address _newBaseInterestAllocator) external {
        address cachedAllocator = getBaseInterestAllocator;
        if (cachedAllocator != address(0)) {
            if (getPendingBaseInterestAllocatorSetTime + UPDATE_WAITING_TIME > block.timestamp) {
                revert TooSoonError();
            }
            if (getPendingBaseInterestAllocator != _newBaseInterestAllocator) {
                revert InvalidInputError();
            }
@>          IBaseInterestAllocator(cachedAllocator).transferAll();
            asset.approve(cachedAllocator, 0);
        }
        asset.approve(_newBaseInterestAllocator, type(uint256).max);

        getBaseInterestAllocator = _newBaseInterestAllocator;
        getPendingBaseInterestAllocator = address(0);
        getPendingBaseInterestAllocatorSetTime = type(uint256).max;

        emit BaseInterestAllocatorSet(_newBaseInterestAllocator);
    }

The current logic is

1. take all the balance of the old BaseInterestAllocator and put it in Pool.
2. change getBaseInterestAllocator to the new BaseInterestAllocator.

If the old BaseInterestAllocator already has a large balance, the balance of the Pool will increase dramatically.

Subsequent users executing reallocate() will get a big bonus getReallocationBonus.

    function reallocate() external nonReentrant returns (uint256) {
        (uint256 currentBalance, uint256 targetIdle) = _reallocate();
        uint256 delta = currentBalance > targetIdle ? currentBalance - targetIdle : targetIdle - currentBalance;
@>      uint256 shares = delta.mulDivDown(totalSupply * getReallocationBonus, totalAssets() * _BPS);

        _mint(msg.sender, shares);

        emit Reallocated(delta, shares);

        return shares;
    }

Assuming old BaseInterestAllocator balance:1 M

shares = 1 M (1 - optimalIdleRange.mid) totalSupply * getReallocationBonus / totalAssets()

Impact

after change BaseInterestAllocator, may pay large getReallocationBonus

Recommended Mitigation

Execute _reallocate() in the confirmBaseInterestAllocator() method without paying any getReallocationBonus.

    function confirmBaseInterestAllocator(address _newBaseInterestAllocator) external {
        address cachedAllocator = getBaseInterestAllocator;
        if (cachedAllocator != address(0)) {
            if (getPendingBaseInterestAllocatorSetTime + UPDATE_WAITING_TIME > block.timestamp) {
                revert TooSoonError();
            }
            if (getPendingBaseInterestAllocator != _newBaseInterestAllocator) {
                revert InvalidInputError();
            }
            IBaseInterestAllocator(cachedAllocator).transferAll();
            asset.approve(cachedAllocator, 0);
        }
        asset.approve(_newBaseInterestAllocator, type(uint256).max);

        getBaseInterestAllocator = _newBaseInterestAllocator;
        getPendingBaseInterestAllocator = address(0);
        getPendingBaseInterestAllocatorSetTime = type(uint256).max;
+       if (cachedAllocator != address(0)) {
+             _reallocate();
+       }
        emit BaseInterestAllocatorSet(_newBaseInterestAllocator);
    }

Assessed type

Context

[c4-judge]

0xA5DF marked the issue as primary issue

[c4-judge]

0xA5DF changed the severity to 2 (Med Risk)

[0xA5DF]

Marking as med since it'll only take the fee part (which is only a small percentage I guess) and I guess also the allocator isn't going to change very often. Tagging @0xend to correct me if I'm wrong

[c4-judge]

0xA5DF marked the issue as selected for report

[c4-judge]

0xA5DF marked the issue as satisfactory

[0xend]

https://github.com/pixeldaogg/florida-contracts/pull/385