code-423n4 / 2024-04-gondi-findings

0 stars 0 forks source link

Loan hashing method is missing protocolFee field, which allows 0 or incorrect protocolFee to pass in multiple flows. ProtocolFees can be lost. #81

Closed c4-bot-5 closed 7 months ago

c4-bot-5 commented 7 months ago

Lines of code

https://github.com/code-423n4/2024-04-gondi/blob/b9863d73c08fcdd2337dc80a8b5e0917e18b036c/src/lib/utils/Hash.sol#L128-L135

Vulnerability details

Impact

Loan hashing method is missing protocolFee field, which allows 0 or incorrect protocolFee to pass in multiple flows. ProtocolFees can be lost.

Proof of Concept

The loan hashing method in Hash.sol is missing the procotolFee field in struct Loan. As a result, all loan hash checks (if(_loan.hash() != _loans[_loanId]){//revert) will be invalid in the sense that any protocolFee input will pass the check. struct Loan has protocolFee field:

//src/interfaces/loans/IMultiSourceLoan.sol
    struct Loan {
        address borrower;
        uint256 nftCollateralTokenId;
        address nftCollateralAddress;
        address principalAddress;
        uint256 principalAmount;
        uint256 startTime;
        uint256 duration;
        Tranche[] tranche;
|>      uint256 protocolFee;
    }

(https://github.com/code-423n4/2024-04-gondi/blob/b9863d73c08fcdd2337dc80a8b5e0917e18b036c/src/interfaces/loans/IMultiSourceLoan.sol#L132) Hashing method doesn't encode procotolFee field:

//src/lib/utils/Hash.sol
    function hash(IMultiSourceLoan.Loan memory _loan) internal pure returns (bytes32) {
...

        return keccak256(
            abi.encode(
                _MULTI_SOURCE_LOAN_HASH,
                _loan.borrower,
                _loan.nftCollateralTokenId,
                _loan.nftCollateralAddress,
                _loan.principalAddress,
                _loan.principalAmount,
                _loan.startTime,
                _loan.duration,
|>                keccak256(trancheHashes) //@audit missing protocolFee field
            )
        );

(https://github.com/code-423n4/2024-04-gondi/blob/b9863d73c08fcdd2337dc80a8b5e0917e18b036c/src/lib/utils/Hash.sol#L128-L135) As a result, multiple flows will have an invalid loan hashing check(_baseLoanChecks()). These flows include refianceFull, refinancePartial,refianceFromLoanExecutionData,addNewTranche, repayLoan, etc. This allows any protocolFee (e.g.0) to pass, protocol might lose fees.

Tools Used

Manual

Recommended Mitigation Steps

Add protocolFee field in loan hashing method.

Assessed type

Other

c4-judge commented 7 months ago

0xA5DF marked the issue as duplicate of #15

c4-judge commented 7 months ago

0xA5DF marked the issue as satisfactory

c4-judge commented 7 months ago

0xA5DF changed the severity to 2 (Med Risk)