Insufficient validation of `maxDueDate` presents a risk for lenders allowing unintended 0% APR loans

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/actions/BuyCreditLimit.sol#L42

Vulnerability details

Description

In the BuyCreditLimit library validateBuyCreditLimit() function properly validates that the maxDueDate is not set before minTenor:

 if (!loanOffer.isNull()) {
            // validate msg.sender
            // N/A

            // validate maxDueDate
            if (params.maxDueDate == 0) {
                revert Errors.NULL_MAX_DUE_DATE();
            }
            if (
 @>             params.maxDueDate < block.timestamp + state.riskConfig.minTenor
            ) {
                revert Errors.PAST_MAX_DUE_DATE(params.maxDueDate);
            }

The maxDueDate allows the lender to do the following:

• create a yield curve with two points ( for simplicity ) : 1 month 2% APR, 1 year 5% APR
• set maxDueDate to 10 months, creating a limit to maximum length of a bid that can be matched with their offer However, considering the current checks it is possible for the lender to set the maxDueDate ( either by mistake, or due to lack of interest in setting such time limit ) to be after the last tenor on the maturity axis of the corresponding yield curve.

Impact

According to the docs:

Size's front-end and back-end systems will help lenders and borrowers borrower to both: [...] and pick the best limit orders given user input preferences

meaning that the off-chain orderbook mechanism will choose the lowest available APR for the borrower's bid. This implies, that a borrower could, either on purpose or by chance, create a bid with just the right length for the lender's offer explained above, which would get him a 0% APR - a free loan. This is because the yield curve uses a linear fitting, so after the last point on the maturity (x) axis the yield value on the yield (y) axis is always 0%.

Proof of Concept

It might be argued that this check is not desirable in the following scenario:

• the maxTenor is 6 months
• lender doesn't want to lend after 2025, so they set maxDueDate to the timestamp corresponding to the beginning of year 2025 However, even considering a scenario where the offer is not fulfilled for such a long time, the lender can always "cancel" their offer by selling their credit through the primary market to other lenders willing to wait.

0. Consider minTenor : 1 hours, maxTenor : 5 * 365 days just like in the Deploy.sol contract
1. A lender creates an offer with two tenors:
   • 1 month 2% APR
   • 1 year 5% APR
2. As they're not interested in setting an "automatic" time limit ( not requiring selling credit to other lenders ), they set maxDueDate to be greater than block.timestamp + tenors[tenors.length - 1].
3. A sell credit limit order which has a tenors[0] equal to 2 years
4. Lender's offer has 0% APR at 2 years, so it's the best match in the eyes of the order book.
5. Borrower gets a free loan, the lender had to unwillingly accept the unreasonable risk.

Recommended Mitigation

Add the following check to validateBuyCreditLimit() function to ensure maxDueDate is not greater than tenors[tenors.length - 1]:

 if (!loanOffer.isNull()) {
            // validate msg.sender
            // N/A

            // validate maxDueDate
            if (params.maxDueDate == 0) {
                revert Errors.NULL_MAX_DUE_DATE();
            }
            if (
-               params.maxDueDate < block.timestamp + state.riskConfig.minTenor
+               params.maxDueDate < block.timestamp + state.riskConfig.minTenor || params.maxDueDate > block.timestamp + state.riskConfig.maxTenor
            ) {
                revert Errors.PAST_MAX_DUE_DATE(params.maxDueDate);
            }

Note that this does NOT limit the lender's ability to cancel their offer through selling credit to other lenders.

Assessed type

Invalid Validation

[aviggiano]

User mistake

[hansfriese]

Unnecessary validation. Users might set a longer maxDueDate if they want and maxTenor will be checked while creating a loan later.

[c4-judge]

hansfriese marked the issue as unsatisfactory: Invalid