code-423n4 / 2024-05-gondi-mitigation-findings

0 stars 0 forks source link

M-12 Unmitigated #30

Open c4-bot-7 opened 3 months ago

c4-bot-7 commented 3 months ago

Lines of code

https://github.com/pixeldaogg/florida-contracts/blob/ac51cc6102fcf5ab274f8812eb585539332431f4/src/lib/LiquidationHandler.sol#L72

Vulnerability details

https://github.com/pixeldaogg/florida-contracts/pull/384 This PR adds getMaxExtension to address the issue of auction times being indefinitely postponed.

The maximum time is: block.timestamp + _liquidationAuctionDuration + getMaxExtension

However, _liquidationAuctionDuration can be modified, with the current constraint being _liquidationAuctionDuration < MAX_AUCTION_DURATION (7 days)

    function updateLiquidationAuctionDuration(uint48 _newDuration) external override onlyOwner {
@>      if (_newDuration < MIN_AUCTION_DURATION || _newDuration > MAX_AUCTION_DURATION) {
            revert InvalidDurationError();
        }
        _liquidationAuctionDuration = _newDuration;

        emit LiquidationAuctionDurationUpdated(_newDuration);
    }

Therefore, it's still possible that: block.timestamp + _liquidationAuctionDuration + getMaxExtension could exceed 7 Days

Recommended Mitigation

It is recommended to restrict _liquidationAuctionDuration + ILoanLiquidator(liquidator).getMaxExtension < MAX_AUCTION_DURATION

    function updateLiquidationAuctionDuration(uint48 _newDuration) external override onlyOwner {
-       if (_newDuration < MIN_AUCTION_DURATION || _newDuration > MAX_AUCTION_DURATION) {
+       if (_newDuration < MIN_AUCTION_DURATION || _newDuration + ILoanLiquidator(liquidator).getMaxExtension  > MAX_AUCTION_DURATION) {    
            revert InvalidDurationError();
        }
        _liquidationAuctionDuration = _newDuration;

        emit LiquidationAuctionDurationUpdated(_newDuration);
    }

Assessed type

Context

alex-ppg commented 3 months ago

The Warden specifies that M-12 has not been sufficiently mitigated as the vulnerability can still resurface after a reconfiguration of the contract due to inadequate limitations in the LiquidationHandler::updateLiquidationAuctionDuration function. I consider the rationale sufficiently elaborated and confirm that M-12 remains unmitigated as it can resurface via future reconfigurations of the system that should be prohibited by the code itself.

c4-judge commented 3 months ago

alex-ppg marked the issue as satisfactory

c4-judge commented 3 months ago

alex-ppg marked the issue as confirmed for report