Potential DoS Attack in BackingManagerP1's rebalance Function

[c4-bot-4]

Lines of code

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

Vulnerability details

Vulnerability Details

The rebalance function in the BackingManagerP1 contract is responsible for maintaining proper collateralization of the RToken system. It can execute trades to adjust collateral levels and is designed to be called periodically.

The function includes a check to prevent multiple rebalances in the same block:

require(
    _msgSender() == address(this) || tradeEnd[kind] < block.timestamp,
    "already rebalancing"
);

However, this check can be exploited by a malicious actor to prevent legitimate rebalancing attempts.

Code Snippet

function rebalance(TradeKind kind) external nonReentrant {
    requireNotTradingPausedOrFrozen();

    // == Refresh ==
    assetRegistry.refresh();

    // DoS prevention:
    // unless caller is self, require that the next auction is not in same block
    require(
        _msgSender() == address(this) || tradeEnd[kind] < block.timestamp,
        "already rebalancing"
    );

    // ... (rest of the function)
}

Impact

An attacker could repeatedly call rebalance with minimal gas, intentionally causing it to fail after setting tradeEnd[kind] to the current block timestamp. This would prevent legitimate calls to rebalance within the same block, potentially delaying critical rebalancing operations and disrupting the system's stability.

Scenario

1. Attacker monitors the mempool for legitimate rebalance transactions.
2. When a legitimate transaction is seen, the attacker front-runs it with their own rebalance call, using minimal gas to ensure it fails after setting tradeEnd[kind].
3. The legitimate rebalance call fails due to the "already rebalancing" check.
4. The attacker repeats this process, consistently blocking rebalance attempts.

Fix

To mitigate this issue, consider implementing a cooldown period for failed rebalance attempts or use a different mechanism to prevent multiple rebalances in the same block. For example:

uint256 public lastRebalanceBlock;

function rebalance(TradeKind kind) external nonReentrant {
    requireNotTradingPausedOrFrozen();

    // == Refresh ==
    assetRegistry.refresh();

    // DoS prevention: Allow only one rebalance attempt per block
    require(block.number > lastRebalanceBlock, "Already attempted rebalance in this block");
    lastRebalanceBlock = block.number;

    // ... (rest of the function)
}

Assessed type

DoS