Summary

The scheduleBatch function allows scheduling operations that depend on pending predecessors, meaning an operation can be scheduled while its predecessor is still unexecuted. However, if the pending predecessor is later canceled or expires, the dependent operation will be permanently stuck, as it requires an execution status that will never be met. Additionally, in executeBatch, the check ensures that the predecessor must be executed before proceeding with the current operation. However, it does not explicitly enforce that the current operation is scheduled only after the predecessor’s execution timestamp. This creates a scenario where an operation may be scheduled to execute before its predecessor, leading to unexpected reverts at execution time rather than at scheduling.

Vulnerability Details

1. Dependency on Pending Predecessors in scheduleBatch:

   • The function allows scheduling operations dependent on predecessors that are merely pending (not yet executed).

   • If the predecessor expires or is canceled, its dependent operation becomes permanently blocked, as it requires an execution condition that can never be met.

2. Lack of Predecessor Execution Time Enforcement in executeBatch:

   • While executeBatch ensures that the predecessor must be executed before the current operation, it does not enforce that the current operation is scheduled only after the predecessor’s execution timestamp.

   • If an operation is scheduled with an execution time earlier than its predecessor’s execution time, the check will not revert until execution, leading to unexpected execution failures rather than preventing incorrect scheduling upfront.

Impact

• Permanent Blocking of Dependent Operations: If a predecessor is expired or canceled, its dependent operations will be permanently stuck, leading to an unusable timelock system.

• Unclear Execution Flow: Operations could be scheduled in an unintended order, leading to a situation where operations cannot execute even if they were initially scheduled correctly.

• Delayed Failure Detection: The lack of a scheduling-time validation in executeBatch leads to failures surfacing at execution time rather than preventing incorrect scheduling at the source.

Tools Used

• Manual code review

• Foundry for testing smart contract execution scenarios

• Static analysis tools (Slither, Mythril) to detect logical inconsistencies

Recommendations

1. Prevent Scheduling on Pending Predecessors:

   • Modify scheduleBatch to ensure that if a predecessor exists, it must already be executed before scheduling a new dependent operation.

   • Example fix:

     if (predecessor != bytes32(0)) {

     if (!isOperationDone(predecessor)) {

     revert PredecessorNotExecuted(predecessor);

     }

     }

     ​

2. Ensure Proper Scheduling Order in executeBatch:

   • Modify the execution check to enforce that the current operation’s scheduled execution time is after the predecessor’s execution time.

   • Example fix:

     if (predecessor != bytes32(0)) {

     if (!isOperationDone(predecessor) || _operations[predecessor].timestamp > op.timestamp) {

     revert InvalidExecutionOrder(predecessor);

     }

     }

     ​

3. Graceful Handling of Expired or Canceled Predecessors:

   • Implement a mechanism to allow force-removal or alternate handling of operations that have unreachable predecessors, such as allowing an admin override or automatic cancellation.

By implementing these fixes, the contract can ensure a more robust and predictable scheduling and execution process, preventing permanent operation blockage and unintended execution failures.