validateOffer() give the wrong _targetIdle to reallocate()

[c4-bot-8]

Lines of code

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

Vulnerability details

Vulnerability details

when validateOffer(), if principalAmount > currentBalance , we need to get back enough from getBaseInterestAllocator to avoid not having enough balance to pay the offer.

    function validateOffer(bytes calldata _offer, uint256 _protocolFee) external override onlyAcceptedCallers {
        if (!isActive) {
            revert PoolStatusError();
        }
        uint256 currentBalance = asset.balanceOf(address(this)) - getAvailableToWithdraw;
        uint256 baseRateBalance = IBaseInterestAllocator(getBaseInterestAllocator).getAssetsAllocated();
        uint256 undeployedAssets = currentBalance + baseRateBalance;
        (uint256 principalAmount, uint256 apr) = IPoolOfferHandler(getUnderwriter).validateOffer(
            IBaseInterestAllocator(getBaseInterestAllocator).getBaseAprWithUpdate(), _offer
        );

        /// @dev Since the balance of the pool includes capital that is waiting to be claimed by the queues,
        ///      we need to check if the pool has enough capital to fund the loan.
        ///      If that's not the case, and the principal is larger than the currentBalance, the we need to reallocate
        ///      part of it.
        if (principalAmount > undeployedAssets) {
            revert InsufficientAssetsError();
        } else if (principalAmount > currentBalance) {
            IBaseInterestAllocator(getBaseInterestAllocator).reallocate(
@>              currentBalance, principalAmount - currentBalance, true
            );
        }
        /// @dev If the txn doesn't revert, we can assume the loan was executed.
        _outstandingValues = _getNewLoanAccounting(principalAmount, _netApr(apr, _protocolFee));
    }

The above call to IBaseInterestAllocator(getBaseInterestAllocator).reallocate(_currentIdle, _targetIdle) with the second argument _targetIdle is wrong. Instead of passing _targetIdle = principalAmount - currentBalance, we should pass _targetIdle = principalAmount directly

The reallocate() method calculates the delta itself internally.

Take AaveUsdcBaseInterestAllocator as an example:

contract AaveUsdcBaseInterestAllocator is IBaseInterestAllocator, Owned {
...
    function reallocate(uint256 _currentIdle, uint256 _targetIdle, bool) external {
        address pool = _onlyPool();
        if (_currentIdle > _targetIdle) {
            uint256 delta = _currentIdle - _targetIdle;
            ERC20(_usdc).transferFrom(pool, address(this), delta);
            IAaveLendingPool(_aavePool).deposit(_usdc, delta, address(this), 0);
        } else {
@>          uint256 delta = _targetIdle - _currentIdle;
            IAaveLendingPool(_aavePool).withdraw(_usdc, delta, address(this));
            ERC20(_usdc).transfer(pool, delta);
        }

        emit Reallocated(_currentIdle, _targetIdle);
    }

Impact

reallocate() incorrectly passes delta, which may cause withdraw to become deposit, resulting in insufficient balance and failed offer.

Recommended Mitigation

    function validateOffer(bytes calldata _offer, uint256 _protocolFee) external override onlyAcceptedCallers {
        if (!isActive) {
            revert PoolStatusError();
        }
        uint256 currentBalance = asset.balanceOf(address(this)) - getAvailableToWithdraw;
        uint256 baseRateBalance = IBaseInterestAllocator(getBaseInterestAllocator).getAssetsAllocated();
        uint256 undeployedAssets = currentBalance + baseRateBalance;
        (uint256 principalAmount, uint256 apr) = IPoolOfferHandler(getUnderwriter).validateOffer(
            IBaseInterestAllocator(getBaseInterestAllocator).getBaseAprWithUpdate(), _offer
        );

        /// @dev Since the balance of the pool includes capital that is waiting to be claimed by the queues,
        ///      we need to check if the pool has enough capital to fund the loan.
        ///      If that's not the case, and the principal is larger than the currentBalance, the we need to reallocate
        ///      part of it.
        if (principalAmount > undeployedAssets) {
            revert InsufficientAssetsError();
        } else if (principalAmount > currentBalance) {
            IBaseInterestAllocator(getBaseInterestAllocator).reallocate(
-               currentBalance, principalAmount - currentBalance, true
+               currentBalance, principalAmount, true
            );
        }
        /// @dev If the txn doesn't revert, we can assume the loan was executed.
        _outstandingValues = _getNewLoanAccounting(principalAmount, _netApr(apr, _protocolFee));
    }

Assessed type

Context

[c4-judge]

0xA5DF marked the issue as duplicate of #63

[c4-judge]

0xA5DF marked the issue as satisfactory