c4-bot-6 commented 7 months ago

Lines of code

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

Vulnerability details

Impact

Inconsistent accounting of undeployedAssets might result in undesired optimal range in the pool

Proof of Concept

undeployedAssets is calculated inconsistently. Currently in _getUndeployedAssets() the protocol collected fees are subtracted, however, in validateOffer, the protocol collected fees are not subtracted.

(1)_getUndeployedAssets():This is called in deployWithdrawalQueue() to calculate proRata liquid assets to the queue.contractAddress.

    function _getUndeployedAssets() private view returns (uint256) {
        return asset.balanceOf(address(this)) + IBaseInterestAllocator(getBaseInterestAllocator).getAssetsAllocated()
|>            - getAvailableToWithdraw - getCollectedFees;
    }

(2)uint256 undeployedAssets: this is manually calculated in validateOffer flow, which is used check whether the pool has enough undeployed Assets to cover loan.principalAmount.

    function validateOffer(bytes calldata _offer, uint256 _protocolFee) external override onlyAcceptedCallers {
...
        uint256 currentBalance = asset.balanceOf(address(this)) - getAvailableToWithdraw;
        uint256 baseRateBalance = IBaseInterestAllocator(getBaseInterestAllocator).getAssetsAllocated();
         //@audit getCollectedFees is not subtracted
|>        uint256 undeployedAssets = currentBalance + baseRateBalance;
        (uint256 principalAmount, uint256 apr) = IPoolOfferHandler(getUnderwriter).validateOffer(
            IBaseInterestAllocator(getBaseInterestAllocator).getBaseAprWithUpdate(), _offer
        );
...

(https://github.com/code-423n4/2024-04-gondi/blob/b9863d73c08fcdd2337dc80a8b5e0917e18b036c/src/lib/pools/Pool.sol#L398) Note that in (2), undeployedAssets are inflated because getCollectedFees are fees protocol collected from liquidation/repayment flows and shouldn't be considered as liquid assets to cover the loan principal amount.

(3)_reallocate(): This also manually calculate total undeployedassets amount, but again didn't account for getCollectedFees. _reaalocate() balances optimal target idle assets ratio by checking currentBalance / total ratio. Here currentBalance should be additionally subtracted by getCollectedFees because fees are set aside and shouldn't be considered idle. This affects optimal range check.

    function _reallocate() private returns (uint256, uint256) {
        /// @dev Balance that is idle and belongs to the pool (not waiting to be claimed)
        uint256 currentBalance = asset.balanceOf(address(this)) - getAvailableToWithdraw;
        if (currentBalance == 0) {
            revert AllocationAlreadyOptimalError();
        }
        uint256 baseRateBalance = IBaseInterestAllocator(getBaseInterestAllocator).getAssetsAllocated();
        uint256 total = currentBalance + baseRateBalance;
        uint256 fraction = currentBalance.mulDivDown(PRINCIPAL_PRECISION, total);
...

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

Inconsistent accounting in various flows may result in incorrect checks or undesirable optimal ranges.