dutchAuctionLength & batchAuctionLength are allowed set to zero.

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/p1/Broker.sol#L221 https://github.com/code-423n4/2024-07-reserve/blob/main/contracts/p1/Broker.sol#L199

Vulnerability details

Vulnerability details

dutchAuctionLength and batchAuctionLength are allowed set to zero in broker contract. So governance set dutchAuctionLength and batchAuctionLength to zero , two auction models can not be initiated since its revert the transaction. So protocol is going to be in insolvant condition.

Proof of Concept

   function setDutchAuctionLength(uint48 newAuctionLength) public governance {
        require(
            newAuctionLength == 0 ||
                (newAuctionLength >= MIN_AUCTION_LENGTH && newAuctionLength <= MAX_AUCTION_LENGTH),
            "invalid dutchAuctionLength"
        );
        emit DutchAuctionLengthSet(dutchAuctionLength, newAuctionLength);
        dutchAuctionLength = newAuctionLength;
    }

Governance able to set dutchAuctionLength to zero. If its zero then openTrade function initiating a newDutchAuction there its reveted due to this reqiure statement.

 function newDutchAuction(
        TradeRequest memory req,
        TradePrices memory prices,
        ITrading caller
    ) private returns (ITrade) {

      ...
      require(dutchAuctionLength != 0, "dutch auctions not enabled");
      ...
}

Same thing happend with batch auction.

 function setBatchAuctionLength(uint48 newAuctionLength) public governance {
        require(
            newAuctionLength == 0 ||
                (newAuctionLength >= MIN_AUCTION_LENGTH && newAuctionLength <= MAX_AUCTION_LENGTH),
            "invalid batchAuctionLength"
        );
        emit BatchAuctionLengthSet(batchAuctionLength, newAuctionLength);
        batchAuctionLength = newAuctionLength;
    }

function newBatchAuction(TradeRequest memory req, address caller) private returns (ITrade) {

    ...
        require(batchAuctionLength != 0, "batch auctions not enabled");
    ...
    }

Impact

In case of governance set dutchAuctionLength and batchAuctionLength are set to zero , Protocol is not able to run the two auction models so eventually prpotocol is going to insolvant state.

Tools Used

Manual Review

Recommended Mitigation Steps

Remove newAuctionLength == 0 from setDutchAuctionLength function and setBatchAuctionLength

Assessed type

DoS

[c4-judge]

thereksfour marked the issue as unsatisfactory: Out of scope

[Yasashari]

@thereksfour Thanks for your effort in judging this contest. I think broker.sol contract is in scope. Please let me know any other reason this is to be out of the scope.

[thereksfour]

https://github.com/code-423n4/2024-07-reserve-findings/issues/50#issuecomment-2310751884 Governance is considered non-malicious

Individual RTokens should assume they can trust veRSR governance. The governing veRSR body should be assumed to be non-malicious