MultiSourceLoan#addNewTranche can be DoS

[c4-bot-8]

Lines of code

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

Vulnerability details

Impact

The addNewTranche function is unavailable due to a DoS attack.

Proof of Concept

The addNewTranche function checks whether the number of _loan.tranches has exceeded MaxTranches.

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

getMaxTranches sets 20 in the test code

abstract contract MultiSourceCommons is TestLoanSetup {
    uint256 internal _maxTranches = 20;
}

An attacker can pass in a small amount of principalAmount, allowing _loan.tranche to increase.

When the length reaches the maximum, the addNewTranche function cannot be called.

An attacker can front-running the addNewTranche function and call it several times before the user, maximizing the array length and causing the user's call to fail.

If getMaxTranches is set to a large value, an attacker can also attack, because executing this function consumes excessive gas when the array length is too large, The caller will fail the call due to insufficient gas.

Tools Used

vscode, manual

Recommended Mitigation Steps

Set the minimum principalAmount. Or use maps to hold Tranche data instead of arrays.

Assessed type

DoS

[c4-judge]

0xA5DF marked the issue as primary issue

[c4-judge]

0xA5DF marked the issue as selected for report

[c4-judge]

0xA5DF marked the issue as satisfactory

[0xend]

https://github.com/pixeldaogg/florida-contracts/pull/376

Only borrower can call this (I think some raised this issue somewhere but not sure).

[minhquanym]

I believed the check for maxTranche in the addNewTranche() is working as expected (it is intended to revert when the number of tranches reach the limit). The core issue should be the function is lacking borrower permission to be called.

[0xend]

that's addressed in a separate issue - sorry if I should have combined them!

[c4-judge]

0xA5DF marked the issue as duplicate of #29

[c4-judge]

0xA5DF marked the issue as partial-25

[0xA5DF]

You're right, that's the root cause here. Duping with 25% credit

[c4-judge]

0xA5DF marked the issue as not selected for report

[c4-judge]

0xA5DF marked the issue as satisfactory

[c4-judge]

0xA5DF changed the severity to 3 (High Risk)