Core Contracts

Summary

The TimelockController::scheduleBatch contains a logical flaw in the predecessor check condition allowing operation to be scheduled even if the predecessor has not been executed. This can lead to incorrect execution order.

Vulnerability Details

The issue lies in scheduleBatch():

This conditional check !isOperationDone(predecessor) && !isOperationPending(predecessor) will always evaluate to false because:

• If the predecessor operation does not exist, both isOperationDone(predecessor) and isOperationPending(predecessor) will return false, making the condition true && true, which is true.

• If the predecessor operation exists, either isOperationDone(predecessor) or isOperationPending(predecessor) will return true, making the condition false.

Impact

Operations may be executed out of order.

Tools Used

Manual review

Recommendations

• Remove this additional check since operations can be scheduled regardless of previous operation's execution status

• Update the condition to correctly check if the predecessor operation has not been executed as:

  if (predecessor != bytes32(0)) {

  if (!isOperationDone(predecessor)) {

  revert PredecessorNotExecuted(predecessor);

  }

  }