Attacker can front-run and pass in empty terms, making it impossible to `confirmTerms()`

[c4-bot-8]

Lines of code

https://github.com/code-423n4/2024-04-gondi/blob/b9863d73c08fcdd2337dc80a8b5e0917e18b036c/src/lib/pools/PoolOfferHandler.sol#L100

Vulnerability details

Impact

In PoolOfferHandler, setting new terms requires two steps (setTerms() and confirmTerms()). The setTerm() function is onlyOwner, but anyone can call the confirmTerms() function. At the end of the confirmTerms() function, pendingTermsSetTime is set to type(uint256).max, preventing the function from being called again.

Since confirmTerms() uses the caller's input to execute the setup, an attacker could input empty __terms to prevent the owner from setting up new terms.

Proof of Concept

// @audit Anyone can front-run and pass in empty terms, making it impossible to confirmTerms
function confirmTerms(TermsKey[] calldata _termKeys, Terms[] calldata __terms) external { 
    if (block.timestamp - pendingTermsSetTime < NEW_TERMS_WAITING_TIME) {
        revert TooSoonError();
    }
    for (uint256 i = 0; i < __terms.length; i++) {
        if (_termKeys[i].duration > getMaxDuration) {
            revert InvalidDurationError();
        }
        uint256 pendingAprPremium = _pendingTerms[_termKeys[i].collection][_termKeys[i].duration][_termKeys[i]
            .maxSeniorRepayment][__terms[i].principalAmount];
        if (pendingAprPremium != __terms[i].aprPremium) {
            revert InvalidTermsError();
        }
        _terms[_termKeys[i].collection][_termKeys[i].duration][_termKeys[i].maxSeniorRepayment][__terms[i]
            .principalAmount] = __terms[i].aprPremium;
        delete _pendingTerms[_termKeys[i].collection][_termKeys[i].duration][_termKeys[i]
            .maxSeniorRepayment][__terms[i].principalAmount];
    }
    pendingTermsSetTime = type(uint256).max;

    emit TermsSet(_termKeys, __terms);
}

In this function, it loops through the input __terms.length list. If an attacker calls confirmTerms() with an empty __terms list, there will be no terms set, and pendingTermsSetTime is still set to type(uint256).max.

As a result, the owner will never be able to set up new terms for the pool because the attacker keeps spamming confirmTerms() when the NEW_TERMS_WAITING_TIME passes after the owner calls setTerms().

Tools Used

Manual Review

Recommended Mitigation Steps

Record the __terms list in the setTerms() function to confirm terms instead of using input from caller.

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]

Fix w/ 59 https://github.com/pixeldaogg/florida-contracts/pull/367