Skip to content

fix(TimelockPolicy): correct ERC-7579 no-op detection encoding #48

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
leekt merged 1 commit into from
Feb 6, 2026
Merged

fix(TimelockPolicy): correct ERC-7579 no-op detection encoding #48

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 11 additions & 21 deletions src/policies/TimelockPolicy.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,20 +299,22 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW
*/
function _isNoOpERC7579Execute(bytes calldata callData) internal view returns (bool) {
// execute(bytes32 mode, bytes calldata executionCalldata)
// Need: 4 (selector) + 32 (mode) + 32 (offset) + 32 (length) + data
if (callData.length < 68) return false;
// ABI layout: 4 (selector) + 32 (mode) + 32 (offset) + 32 (length) + data
if (callData.length < 100) return false;

// Decode the offset to executionCalldata (should be 32)
// Offset to executionCalldata: 2 head slots (mode + offset) = 64
uint256 offset = uint256(bytes32(callData[36:68]));
if (offset != 32) return false;
if (offset != 64) return false;

// Decode the length of executionCalldata
if (callData.length < 100) return false;
uint256 execDataLength = uint256(bytes32(callData[68:100]));

// For single execution mode, executionCalldata format is:
// target (20 bytes) + value (32 bytes) + calldata (variable)
if (execDataLength < 52) return false;
// ERC-7579 single execution uses compact format (no length prefix):
// executionCalldata = abi.encodePacked(target, value, calldata)
// target (20 bytes) + value (32 bytes) = 52 bytes with no inner calldata
if (execDataLength != 52) return false;

if (callData.length < 152) return false;

// Extract target address (first 20 bytes of executionCalldata)
address target = address(bytes20(callData[100:120]));
Expand All @@ -324,19 +326,7 @@ contract TimelockPolicy is PolicyBase, IStatelessValidator, IStatelessValidatorW
uint256 value = uint256(bytes32(callData[120:152]));

// Value must be 0
if (value != 0) return false;

// Check calldata length (remaining bytes should indicate empty calldata)
// executionCalldata = target(20) + value(32) + calldataLength(32) + calldata
if (callData.length < 184) {
// If we don't have enough for calldata length field, it's malformed
return false;
}

uint256 innerCalldataLength = uint256(bytes32(callData[152:184]));

// Inner calldata must be empty
return innerCalldataLength == 0;
return value == 0;
}

/**
Expand Down
76 changes: 24 additions & 52 deletions test/btt/Timelock.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -701,15 +701,11 @@ contract TimelockTest is Test {

function test_GivenTargetIsSelfAndValueIsZeroAndInnerCalldataIsEmpty() external whenDetectingERC7579ExecuteNoop {
// it should be detected as noop
bytes memory callData = abi.encodePacked(
IERC7579Execution.execute.selector,
bytes32(0), // mode
bytes32(uint256(32)), // offset
bytes32(uint256(84)), // execDataLength (20+32+32)
bytes20(WALLET), // target = self
bytes32(uint256(0)), // value = 0
bytes32(uint256(0)) // innerCalldataLength = 0
);
// ERC-7579 compact format: abi.encodePacked(target, value) = 52 bytes
bytes memory executionCalldata = abi.encodePacked(bytes20(WALLET), uint256(0));

bytes memory callData =
abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), executionCalldata);

bytes memory sig = _createProposalSignature("proposal", 0);
PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, callData, 0, sig);
Expand All @@ -725,15 +721,11 @@ contract TimelockTest is Test {
whenDetectingERC7579ExecuteNoop
{
// it should be detected as noop
bytes memory callData = abi.encodePacked(
IERC7579Execution.execute.selector,
bytes32(0), // mode
bytes32(uint256(32)), // offset
bytes32(uint256(84)), // execDataLength
bytes20(address(0)), // target = address(0)
bytes32(uint256(0)), // value = 0
bytes32(uint256(0)) // innerCalldataLength = 0
);
// ERC-7579 compact format: abi.encodePacked(target, value) = 52 bytes
bytes memory executionCalldata = abi.encodePacked(bytes20(address(0)), uint256(0));

bytes memory callData =
abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), executionCalldata);

bytes memory sig = _createProposalSignature("proposal", 1);
PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, callData, 0, sig);
Expand All @@ -756,15 +748,14 @@ contract TimelockTest is Test {
assertEq(result, SIG_VALIDATION_FAILED, "Too short for offset should not be noop");
}

function test_GivenOffsetIsNot32() external whenDetectingERC7579ExecuteNoop {
function test_GivenOffsetIsNot64() external whenDetectingERC7579ExecuteNoop {
// it should not be detected as noop
bytes memory callData = abi.encodePacked(
IERC7579Execution.execute.selector,
bytes32(0), // mode
bytes32(uint256(64)), // wrong offset (should be 32)
bytes32(uint256(32)), // wrong offset (should be 64)
bytes32(uint256(52)), // length
WALLET,
uint256(0),
bytes20(WALLET),
uint256(0)
);

Expand All @@ -781,7 +772,7 @@ contract TimelockTest is Test {
bytes memory callData = abi.encodePacked(
IERC7579Execution.execute.selector,
bytes32(0), // mode
bytes32(uint256(32)) // offset
bytes32(uint256(64)) // offset
// missing length and data
);

Expand All @@ -793,26 +784,26 @@ contract TimelockTest is Test {
assertEq(result, SIG_VALIDATION_FAILED, "Too short for length should not be noop");
}

function test_GivenExecDataLengthIsLessThan52() external whenDetectingERC7579ExecuteNoop {
// it should not be detected as noop
function test_GivenExecDataLengthIsNot52() external whenDetectingERC7579ExecuteNoop {
// it should not be detected as noop (length 20 != 52)
bytes memory callData = abi.encodePacked(
IERC7579Execution.execute.selector,
bytes32(0), // mode
bytes32(uint256(32)), // offset
bytes32(uint256(20)) // length only 20 (need at least 52)
bytes32(uint256(64)), // offset
bytes32(uint256(20)) // length only 20 (must be exactly 52)
);

PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, callData, 0, "");

vm.prank(WALLET);
uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp);

assertEq(result, SIG_VALIDATION_FAILED, "Exec data too short should not be noop");
assertEq(result, SIG_VALIDATION_FAILED, "Exec data length != 52 should not be noop");
}

function test_GivenTargetIsNotSelfOrZero() external whenDetectingERC7579ExecuteNoop {
// it should not be detected as noop
bytes memory executionCalldata = abi.encodePacked(ATTACKER, uint256(0), uint256(0));
bytes memory executionCalldata = abi.encodePacked(bytes20(ATTACKER), uint256(0));

bytes memory callData =
abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), executionCalldata);
Expand All @@ -827,7 +818,7 @@ contract TimelockTest is Test {

function test_GivenValueIsNonzero() external whenDetectingERC7579ExecuteNoop {
// it should not be detected as noop
bytes memory executionCalldata = abi.encodePacked(WALLET, uint256(1 ether), uint256(0));
bytes memory executionCalldata = abi.encodePacked(bytes20(WALLET), uint256(1 ether));

bytes memory callData =
abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), executionCalldata);
Expand All @@ -840,28 +831,9 @@ contract TimelockTest is Test {
assertEq(result, SIG_VALIDATION_FAILED, "Non-zero value should not be noop");
}

function test_GivenCalldataIsShorterThan184Bytes() external whenDetectingERC7579ExecuteNoop {
// it should not be detected as noop
bytes memory callData = abi.encodePacked(
IERC7579Execution.execute.selector,
bytes32(0), // mode
bytes32(uint256(32)), // offset
bytes32(uint256(52)), // execDataLength (exactly 52, no room for inner calldata length)
WALLET, // 20 bytes
uint256(0) // 32 bytes = 52 total
);

PackedUserOperation memory userOp = _createUserOpWithCalldata(WALLET, callData, 0, "");

vm.prank(WALLET);
uint256 result = timelockPolicy.checkUserOpPolicy(POLICY_ID, userOp);

assertEq(result, SIG_VALIDATION_FAILED, "Too short for inner calldata length should not be noop");
}

function test_GivenInnerCalldataLengthIsNonzero() external whenDetectingERC7579ExecuteNoop {
// it should not be detected as noop
bytes memory executionCalldata = abi.encodePacked(WALLET, uint256(0), uint256(10));
function test_GivenExecDataLengthGreaterThan52() external whenDetectingERC7579ExecuteNoop {
// it should not be detected as noop (has inner calldata)
bytes memory executionCalldata = abi.encodePacked(bytes20(WALLET), uint256(0), hex"deadbeef");

bytes memory callData =
abi.encodeWithSelector(IERC7579Execution.execute.selector, bytes32(0), executionCalldata);
Expand Down