c4-bot-7 commented 3 months ago

Lines of code

https://github.com/pixeldaogg/florida-contracts/blob/10d48b51313496c41c886cd46e610b627ef159aa/src/lib/loans/LoanManagerParameterSetter.sol#L70

Vulnerability details

Impact

The offer handler is a crucial part of the LoanManager. It handles what loan terms are allowed to taken from the Pool, affect the risk and interest of Pool's users directly. To facilitate changes made by the owner, the protocol uses a two-step process involving two functions: setOfferHandler() and confirmOfferHandler(). These are located in the LoanManagerParameterSetter.

However, the function LoanManager.updateOfferHandler() is immediately executed within setOfferHandler(), not confirmOfferHandler(). Consequently, the updated offer handler takes effect instantly without needing confirmation, which undermines the two-step process's intent.

Proof of Concept

function setOfferHandler(address __offerHandler) external onlyOwner {
    // ...
    getProposedOfferHandler = __offerHandler;
    getProposedOfferHandlerSetTime = block.timestamp;
    // @audit should wait for confirmOfferHandler before updating in LoanManager
    ILoanManager(getLoanManager).updateOfferHandler(__offerHandler); 
      // ...
}

/// @notice Confirm the OfferHandler contract.
/// @param __offerHandler The new OfferHandler address.
function confirmOfferHandler(address __offerHandler) external onlyOwner {
    // ...

    getOfferHandler = __offerHandler;
    getProposedOfferHandler = address(0);
    getProposedOfferHandlerSetTime = type(uint256).max;

    // ...
}

Tools Used

Manual Review

Recommended Mitigation Steps

Instead of calling LoanManager.updateOfferHandler() within setOfferHandler(), it should be executed within confirmOfferHandler().

Assessed type

Other

0xend commented 3 months ago

duplicate w 33

c4-judge commented 3 months ago

alex-ppg marked the issue as duplicate of #33