Satellite chains can't execute onlyGovernance functions
| Title | Satellite chains can't execute onlyGovernance functions |
|---|---|
| Reward | $5668, 3 dups |
| Contest | Summer.fi - 29 Sep 2025 on Sherlock |
| Author | EgisSecurity |
| Context | Wrong Call Path |
This issue is hard to explain on its own, so here is the background. The first thing to go over is the OpenZeppelin GovernorTimelockControl.sol, and how it interacts with the Timelock. As can be seen below, the governor contract calls the timelock, which then adds the necessary delay.
/**
* @dev Overridden version of the {Governor-_executeOperations} function that runs the already queued proposal
* through the timelock.
*/
function _executeOperations(
uint256 proposalId,
address[] memory targets,
uint256[] memory values,
bytes[] memory calldatas,
bytes32 descriptionHash
) internal virtual override {
// execute
_timelock.executeBatch{value: msg.value}(targets, values, calldatas, 0, _timelockSalt(descriptionHash));
// cleanup for refund
delete _timelockIds[proposalId];
}In the Summer.fi contest, cross-chain Satellites (Governors on other chains) run their schedule and execute steps directly through the Timelock, rather than through their Governor flow.
function _queueCrossChainProposal(
uint256 proposalId,
address[] memory targets,
uint256[] memory values,
bytes[] memory calldatas,
bytes32 descriptionHash
) internal onlySatelliteChain returns (uint256) {
// Satellite-only queueing of received operations into the local timelock controller
uint48 eta = _queueOperations(
proposalId,
targets,
values,
calldatas,
descriptionHash
);
emit ProposalQueued(proposalId, uint256(eta));
return proposalId;
}The problem with this approach is that when the Timelock executes any onlyGovernance function on the Satellite, it is executed directly by calling it from the Satellite::_queueCrossChainProposal() function, instead of using its inherited Governor::execute() function.
This matters because the onlyGovernance modifier on the Satellite requires the call to have been placed in a queue (see the OpenZeppelin contract).
modifier onlyGovernance() {
_checkGovernance();
_;
}
function _checkGovernance() internal virtual {
if (_executor() != _msgSender()) {
revert GovernorOnlyExecutor(_msgSender());
}
if (_executor() != address(this)) {
bytes32 msgDataHash = keccak256(_msgData());
// loop until popping the expected operation - throw if deque is empty (operation not authorized)
while (_governanceCall.popFront() != msgDataHash) {}
}
}But this only happens when execution is routed through the Governor::execute() function (which, again, calls the Timelock, see OZ), not directly to the Timelock.
function execute(
address[] memory targets,
uint256[] memory values,
bytes[] memory calldatas,
bytes32 descriptionHash
) public payable virtual returns (uint256) {
uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
_validateStateBitmap(
proposalId,
_encodeStateBitmap(ProposalState.Succeeded) | _encodeStateBitmap(ProposalState.Queued)
);
// mark as executed before calls to avoid reentrancy
_proposals[proposalId].executed = true;
// before execute: register governance call in queue.
if (_executor() != address(this)) {
for (uint256 i = 0; i < targets.length; ++i) {
if (targets[i] == address(this)) {
_governanceCall.pushBack(keccak256(calldatas[i]));
}
}
}
_executeOperations(proposalId, targets, values, calldatas, descriptionHash);
// after execute: cleanup governance call queue.
if (_executor() != address(this) && !_governanceCall.empty()) {
_governanceCall.clear();
}
emit ProposalExecuted(proposalId);
return proposalId;
}As a result, certain functions on the Satellite governor contract cannot be executed from a cross-chain call.
Conclusion
This finding would earn you $5668. It is not especially difficult, but it needs familiarity with OpenZeppelin's governance contracts, or at least running some tests. It lands as medium severity because functionality is broken.