The getMaxTranches check can be bypassed

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2024-04-gondi/blob/b9863d73c08fcdd2337dc80a8b5e0917e18b036c/src/lib/loans/MultiSourceLoan.sol#L377

Vulnerability details

Impact

In MultiSourceLoan, each loan has a limited number of tranches. This limit is held in the getMaxTranches variable. When users want to add a new tranche, this limit is checked to ensure a loan cannot have more tranches than the getMaxTranches limit. The check is presented in addNewTranche() function.

However, this check is absent in the emitLoan() function, making it easy to bypass the other check in function addNewTranche().

Proof of Concept

function addNewTranche(
    RenegotiationOffer calldata _renegotiationOffer,
    Loan memory _loan,
    bytes calldata _renegotiationOfferSignature
) external nonReentrant returns (uint256, Loan memory) {
    ...
    if (_loan.tranche.length == getMaxTranches) {
        revert TooManyTranchesError();
    }
    ...
}

As shown above, the check uses the equal comparator. So, users could create a loan with getMaxTranches + 1 tranches through the emitLoan() function, then they can always add more tranches. This is because the length of _loan.tranche.length is already larger than getMaxTranches and continues to grow after calling addNewTranche().

Tools Used

Manual Review

Recommended Mitigation Steps

Include a getMaxTranches check in the emitLoan() function and modify the check to

if (_loan.tranche.length >= getMaxTranches) {
    revert TooManyTranchesError();
}

Assessed type

Other

[c4-judge]

0xA5DF marked the issue as duplicate of #80

[c4-judge]

0xA5DF marked the issue as satisfactory