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

0 stars 0 forks source link

`Loan` struct hash does not include protocol fee #53

Closed c4-bot-3 closed 7 months ago

c4-bot-3 commented 7 months ago

Lines of code

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

Vulnerability details

Impact

In Gondi, when a new loan is issued, all the loan parameters are represented in the Loan struct. Only the hash value of this struct is stored on-chain. Therefore, when users want to interact with an existing loan, they need to input the entire Loan struct. This is necessary when users want to refinance, or borrowers want to repay the loan.

However, the protocolFee is not included in the Loan struct hash. As a result, users could input an arbitrary protocolFee value when they interact with an existing loan to avoid the protocol fee. For example, when a borrower repays the loan, the protocolFee is taken from the interest paid by the borrower. But since the loan.protocolFee could be input as 0, they can avoid this fee.

if (withProtocolFee) {
    thisProtocolFee = newInterest.mulDivUp(loan.protocolFee, _PRECISION);
    totalProtocolFee += thisProtocolFee;
}

Proof of Concept

Observe the function used to hash the Loan struct and the Loan struct itself. It is clear that the protocolFee is not included in the Loan struct hash.

struct Loan {
    address borrower;
    uint256 nftCollateralTokenId;
    address nftCollateralAddress;
    address principalAddress;
    uint256 principalAmount;
    uint256 startTime;
    uint256 duration;
    Tranche[] tranche;
    uint256 protocolFee;
}

function hash(IMultiSourceLoan.Loan memory _loan) internal pure returns (bytes32) {
    bytes memory trancheHashes;
    for (uint256 i; i < _loan.tranche.length;) {
        trancheHashes = abi.encodePacked(trancheHashes, _hashTranche(_loan.tranche[i]));
        unchecked {
            ++i;
        }
    }
    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 protocolFee is missing
    );
}

Tools Used

Manual Review

Recommended Mitigation Steps

Include protocolFee in the Loan struct hash.

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)