code-423n4 / 2024-07-reserve-findings

1 stars 0 forks source link

Running external rebalancing might be delayed even after a Dutch auction has already been settled #104

Closed howlbot-integration[bot] closed 1 month ago

howlbot-integration[bot] commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/BackingManager.sol#L116

Vulnerability details

Impact

It will be unable to run external rebalancing in backing manager until tradeEnd[DUTCH_AUCTION] expires, even when a Dutch auction has already been settled.

Proof of Concept

To prevent an external rebalancing in the same block of the settlement, the BackingManager has the following restriction:

    function rebalance(TradeKind kind) external nonReentrant {
        ... ...
        require(
            _msgSender() == address(this) || tradeEnd[kind] < block.timestamp, 
            "already rebalancing"
        );
        ... ...
    }

a Dutch auction is considered settled as soon as any bidder places a bid, regardless of the remaining auction period.

Let's assume the following scenario.

  1. There is a collateral default, so starts a dutch auction of 6 days (the maximum auction length is 7 days).
  2. After 3 days are passed, the auction is settled by a bidder, and as there's no more collateral default, the next rebalancing is just returned without running a new auction.
  3. In the next day, a new collateral default happens, so when trying to run rebalancing by external call, it is reverted by the restriction of tradeEnd[DUTCH_AUCTION] because 2 days are remaining.

Therefore, during the remaining auction period, calling the rebalance() function will be reverted by the above restriction.

Tools Used

Manual Review

Recommended Mitigation Steps

I would suggest updating tradeEnd[DUTCH_AUCTION] in the settleTrade() function:

    function settleTrade(IERC20 sell) public override(ITrading, TradingP1) returns (ITrade trade) {
        delete tokensOut[sell];
        trade = super.settleTrade(sell); // nonReentrant
+       if (trade.KIND() == TradeKind.BATCH_AUCTION && block.timestamp < tradeEnd[trade.KIND()])
+           tradeEnd[trade.KIND()] = block.timestamp;
        ... ...
    }

Assessed type

Invalid Validation

c4-judge commented 1 month ago

thereksfour changed the severity to QA (Quality Assurance)

c4-judge commented 1 month ago

This previously downgraded issue has been upgraded by thereksfour

c4-judge commented 1 month ago

thereksfour marked the issue as satisfactory