Submission Details
Severity:
According comment scheduleBatch Function Logic does not work correctly
Summary
In TimelockController.sol According comment section scheduleBatch Function Logic does not work correctly. Because in comment have predecessor ID of operation that must be executed before. But in Function logic predecessor IDis checking wrongly. In Code That Not matter predecessor IDis executed or not , function will schedule batch correctly Even predecessor ID is not executed.
Vulnerability Details
/**
* @notice Schedules a batch of operations
* @dev Only callable by addresses with PROPOSER_ROLE
* @param targets Target addresses for calls
* @param values ETH values for calls
* @param calldatas Calldata for calls
@>>> * @param predecessor ID of operation that must be executed before
* @param salt Random value for operation ID
* @param delay Timelock delay for this operation
* @return id Operation ID
*/
function scheduleBatch(
address[] calldata targets,
uint256[] calldata values,
bytes[] calldata calldatas,
bytes32 predecessor,
bytes32 salt,
uint256 delay
) external override onlyRole(PROPOSER_ROLE) returns (bytes32) {
// Input validation: check if the number of targets, values, and calldatas are the same
if (targets.length == 0 || targets.length != values.length || targets.length != calldatas.length) {
revert InvalidTargetCount();
}
// Check if the delay is within the allowed range
if (delay < _minDelay || delay > _maxDelay) {
revert InvalidDelay(delay);
}
// Check predecessor if specified
if (predecessor != bytes32(0)) {
@>>> - if (!isOperationDone(predecessor) && !isOperationPending(predecessor)) @> Wrong Logic
@>>> + if (!isOperationDone(predecessor)) @> Correct Logic
{
revert PredecessorNotExecuted(predecessor);
}
}
bytes32 id = hashOperationBatch(targets, values, calldatas, predecessor, salt);
if (_operations[id].timestamp != 0) revert OperationAlreadyScheduled(id);
uint256 timestamp = block.timestamp + delay;
_operations[id] = Operation({
timestamp: timestamp.toUint64(),
executed: false
});
emit OperationScheduled(id, targets, values, calldatas, predecessor, salt, delay);
return id;
}
Impact
Function not work as commented. That lead to unintetional behaviour of function.
Tools Used
manual review
Recommendations
Correct predecessor IDchecking logic in the function.
Updates
Lead Judging Commences
inallhonesty 5 months ago
Submission Judgement Published Assigned finding tags:
TimelockController::scheduleBatch checks if predecessor is pending OR executed rather than requiring execution as per comment, allowing scheduling before predecessor executes
inallhonesty 5 months ago
Submission Judgement Published Assigned finding tags:
TimelockController::scheduleBatch checks if predecessor is pending OR executed rather than requiring execution as per comment, allowing scheduling before predecessor executes
Prize pool breakdown
Total prize
77,280 USDC
nSLOC
3,86420 USDC / LOC
74,000
3,280
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.