Incorrect selector extraction always returns zero
Description
The extractSelector function in ManagementAccountLib incorrectly applies a shr(224, ...) operation before assigning to a bytes4 return type:
function extractSelector(bytes memory data) internal pure returns (bytes4 selector) {
if (data.length < 4) return bytes4(0);
assembly {
! selector := shr(224, mload(add(data, 32)))
}
}The shr(224, ...) shifts the selector bytes to the right, moving them out of the bytes4 range entirely, resulting in bytes4(0). The shift is unnecessary since casting to bytes4 already extracts the first four bytes from the loaded bytes32 word.
This function is used in executeApprovedServiceAction and _executeServiceCalls to emit the ServiceCallExecuted event:
function _executeServiceCalls(
address service,
IService.Call[] memory calls
)
internal
returns (bytes[] memory results)
{
[...]
// Emit events for each call
uint256 length = calls.length;
for (uint256 i; i < length; ++i) {
bytes4 selector = ManagementAccountLib.extractSelector(calls[i].callData);
! emit ServiceCallExecuted(service, calls[i].target, selector);
}
}
function executeApprovedServiceAction(
uint256 actionId,
string calldata action
)
external
nonReentrant
returns (bytes[] memory results)
{
[...]
for (uint256 i; i < length; ++i) {
IService.Call memory call = calls[i];
[...]
bytes4 selector = ManagementAccountLib.extractSelector(call.callData);
! emit ServiceCallExecuted(service, call.target, selector);
results[i] = response;
}
[...]
}Due to this bug, the event always emits bytes4(0) instead of the actual function selector.
Impact
This breaks off-chain monitoring that relies on these events to track which functions are executed during approved service actions.
Recommendations
Remove the unnecessary shr(224, ...) operation from the assembly block:
function extractSelector(bytes memory data) internal pure returns (bytes4 selector) {
if (data.length < 4) return bytes4(0);
assembly {
- selector := shr(224, mload(add(data, 32)))
+ selector := mload(add(data, 32))
}
}Remediation
This issue has been acknowledged by Hyperbeat, and a fix was implemented in commit d7b521b0↗.