Submitted by oakcobalt
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.
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 doesnt verify the caller is a loan contract (_isLoanContract(caller)``==true).
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.
In Pool::validateOffer, consider adding a check to ensure _isLoanContract(msg.sender)``==true.
Invalid Validation
0xend (Gondi) acknowledged and commented:
0xA5DF (judge) commented:
Gondi mitigated:
Status: Mitigation confirmed. Full details in reports from oakcobalt, minhquanym and bin2chen.
