Any liquidators can pretend to be a loan contract to validate offers, due to insufficient validation

mediumCode4rena

Lines of code

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

Vulnerability details

Impact

Any liquidators can pretend to be a loan contract to validate offers, due to insufficient validation

Proof of Concept

Accepted callers in a loan manager (e.g. Pool.sol) can be either liquidators or loan contracts.

And only loan contracts should validate offers during a loan creation. However, the current access control check is insufficient in pool::validateOffer, which allows liquidators to pretend to be a loan contract, and directly modify storage (__outstandingValues) bypassing additional checks and accounting in a loan contract.

solidity📋
//src/lib/pools/Pool.sol
    //@audit onlyAcceptedCallers only doesn't ensure caller is a loan contract
|>    function validateOffer(bytes calldata _offer, uint256 _protocolFee) external override onlyAcceptedCallers {
        if (!isActive) {
            revert PoolStatusError();
        }
...

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

Current access control check(onlyAcceptedCallers) only ensures caller is accepted caller but doesn't verify the caller is a loan contract (_isLoanContract(caller)==true).

solidity📋
//src/lib/loans/LoanManager.sol
    modifier onlyAcceptedCallers() {
        if (!_acceptedCallers.contains(msg.sender)) {
            revert CallerNotAccepted();
        }
        _;
    }

When a liquidator calls validateOffer, they can provide a fabricated bytes calldata _offer and uint256 _protocolFee bypassing additional checks and state accounting in a loan contract. For example, in MultiSourceLoan.sol- emitLoan, extra checks are implemented on LoanExecutionData to verify borrower and lender signatures and offer expiration timestamp as well as transfer collateral NFT tokens and recoding loan to storage. All of the above can be skipped if a liquidator directly call validateOffer and modify __outstandingValues without token transfer.

Tools Used

Manual

Recommended Mitigation Steps

In Pool::validateOffer, consider adding a check to ensure _isLoanContract(msg.sender)==true

Assessed type

Invalid Validation