validateOffer() reentry to manipulate exchangeRate

[c4-bot-10]

Lines of code

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

Vulnerability details

Vulnerability details

The current mechanism of validateOffer() is to first book _outstandingValues to increase, but assets.balanceOf(address(this)) doesn't decrease immediately

    function validateOffer(bytes calldata _offer, uint256 _protocolFee) external override onlyAcceptedCallers {
..

        /// @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));
    }

I.e.: After this method is called, _getUndeployedAssets() is unchanged, but _getTotalOutstandingValue() is increased, so totalAssets() is increased, but totalSupply is unchanged, so exchangeRate is get bigger.

Originally, it was expected that after that, the Pool balance would be transferred at MultiSourceLoan, so _getUndeployedAssets() becomes smaller and exchangeRate returns to normal.

But if it's possible to do callback malicious logic before MultiSourceLoan transfers away the Pool balance, it's possible to take advantage of this exchangeRate that becomes larger

Example: Suppose: _getUndeployedAssets() = 1000 _getTotalOutstandingValue() = 1000 totalSupply = 2000 so: totalAssets() = 2000 exchangeRate = 1:1

1. alice call MultiSourceLoan.emitLoan()
   • offer.lender = pool
   • offer.principalAmount = 500
   • offer.validators = CustomContract -> for callback
2. emitLoan() -> Pool.validateOffer()
   • _getUndeployedAssets() = 1000 (no change)
   • _getTotalOutstandingValue() = 1000 + 500 = 1500 (more 500)
   • totalAssets() = 2500
   • exchangeRate = 1.25 : 1
3. emitLoan() -> _checkValidators() -> CustomContract.validateOffer()
   • in CustomContract.validateOffer() call pool.redeem(shares) use exchangeRate = 1.25 : 1 to get more assets
4. emitLoan() -> asset.safeTransferFrom(pool,receiver,500)
   • _getUndeployedAssets() -= 500
   • exchangeRate Expect to return to normal

Impact

Manipulating the exchangeRate to redeem additional assets

Recommended Mitigation

In validateOffer(), restrict offer.validators to be an empty array to avoid callbacks.

Assessed type

Context

[c4-judge]

0xA5DF marked the issue as primary issue

[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/381