Open c4-bot-1 opened 7 months ago
0xA5DF marked the issue as primary issue
This is a low one given these contracts must all live within our ecosystem and be whitelisted. Given there's alreayd that trust assumption, not doing that check to save some gas on an extra state read.
@0xend Sponsors can only use these labels: sponsor confirmed, sponsor disputed, sponsor acknowledged.
0xA5DF marked the issue as selected for report
0xA5DF marked the issue as satisfactory
This is a low one given these contracts must all live within our ecosystem and be whitelisted
That makes sense. However, I'm judging this based on the info that was present to the wardens in the README and docs. Given that that trust assumption wasn't noted there I'm going to sustain med severity.
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.(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).When a liquidator calls
validateOffer
, they can provide a fabricatedbytes calldata _offer
anduint256 _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 callvalidateOffer
and modify__outstandingValues
without token transfer.Tools Used
Manual
Recommended Mitigation Steps
In Pool::validateOffer, consider adding a check to ensure
_isLoanContract(msg.sender)
==trueAssessed type
Invalid Validation