From 7f72f592aab61a13ac3dd51ffe901d2a9fee2179 Mon Sep 17 00:00:00 2001 From: Sasha Mitchell Date: Sat, 8 Aug 2026 11:19:51 +0700 Subject: [PATCH] fix(enforcers): reject non-zero native value on ERC20 streaming and ownership transfer ERC20StreamingEnforcer and OwnershipTransferEnforcer decoded executions as (target,, callData) and never constrained native value. Token/ownership-scoped delegations could therefore accompany unbounded ETH on payable transfer / transferOwnership targets. Match ApprovalRevocationEnforcer / MultiTokenPeriodEnforcer (and open #195 siblings) by requiring value == 0. Signed-off-by: Sasha Mitchell --- src/enforcers/ERC20StreamingEnforcer.sol | 3 ++- src/enforcers/OwnershipTransferEnforcer.sol | 3 ++- test/enforcers/ERC20StreamingEnforcer.t.sol | 11 +++++++++++ test/enforcers/OwnershipTransferEnforcer.t.sol | 18 ++++++++++++++++++ 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/enforcers/ERC20StreamingEnforcer.sol b/src/enforcers/ERC20StreamingEnforcer.sol index 6dbae52b..5512d12e 100644 --- a/src/enforcers/ERC20StreamingEnforcer.sol +++ b/src/enforcers/ERC20StreamingEnforcer.sol @@ -155,8 +155,9 @@ contract ERC20StreamingEnforcer is CaveatEnforcer { ) private { - (address target_,, bytes calldata callData_) = _executionCallData.decodeSingle(); + (address target_, uint256 value_, bytes calldata callData_) = _executionCallData.decodeSingle(); + require(value_ == 0, "ERC20StreamingEnforcer:invalid-value"); require(callData_.length == 68, "ERC20StreamingEnforcer:invalid-execution-length"); (address token_, uint256 initialAmount_, uint256 maxAmount_, uint256 amountPerSecond_, uint256 startTime_) = diff --git a/src/enforcers/OwnershipTransferEnforcer.sol b/src/enforcers/OwnershipTransferEnforcer.sol index 41ebf6d5..83f491c4 100644 --- a/src/enforcers/OwnershipTransferEnforcer.sol +++ b/src/enforcers/OwnershipTransferEnforcer.sol @@ -72,8 +72,9 @@ contract OwnershipTransferEnforcer is CaveatEnforcer { pure returns (address newOwner_) { - (address target_,, bytes calldata callData_) = _executionCallData.decodeSingle(); + (address target_, uint256 value_, bytes calldata callData_) = _executionCallData.decodeSingle(); + require(value_ == 0, "OwnershipTransferEnforcer:invalid-value"); require(callData_.length == 36, "OwnershipTransferEnforcer:invalid-execution-length"); bytes4 selector_ = bytes4(callData_[0:4]); diff --git a/test/enforcers/ERC20StreamingEnforcer.t.sol b/test/enforcers/ERC20StreamingEnforcer.t.sol index e4b7b2cf..2e129963 100644 --- a/test/enforcers/ERC20StreamingEnforcer.t.sol +++ b/test/enforcers/ERC20StreamingEnforcer.t.sol @@ -63,6 +63,17 @@ contract ERC20StreamingEnforcerTest is CaveatEnforcerBaseTest { } //////////////////// Error / Revert Tests ////////////////////// + + /// @notice Reverts if a streaming ERC20 transfer execution carries non-zero native value. + function test_revertOnNonZeroValue() public { + bytes memory terms_ = _encodeTerms(address(basicERC20), 10 ether, 100 ether, 1 ether, block.timestamp); + bytes memory callData_ = _encodeERC20Transfer(bob, 1 ether); + bytes memory execData_ = _encodeSingleExecution(address(basicERC20), 1, callData_); + + vm.expectRevert(bytes("ERC20StreamingEnforcer:invalid-value")); + erc20StreamingEnforcer.beforeHook(terms_, bytes(""), singleDefaultMode, execData_, bytes32(0), address(0), alice); + } + /** * @notice Ensures it reverts if `_terms.length != 148`. */ diff --git a/test/enforcers/OwnershipTransferEnforcer.t.sol b/test/enforcers/OwnershipTransferEnforcer.t.sol index 26de10f2..c62004cd 100644 --- a/test/enforcers/OwnershipTransferEnforcer.t.sol +++ b/test/enforcers/OwnershipTransferEnforcer.t.sol @@ -63,6 +63,24 @@ contract OwnershipTransferEnforcerTest is CaveatEnforcerBaseTest { ////////////////////// Errors ////////////////////// + // Reverts if ownership transfer execution carries non-zero native value + function test_revertOnNonZeroValue() public { + address newOwner = address(0x5678); + bytes memory terms_ = abi.encodePacked(mockContract); + transferOwnershipExecution = Execution({ + target: mockContract, + value: 1, + callData: abi.encodeWithSelector(bytes4(keccak256("transferOwnership(address)")), newOwner) + }); + transferOwnershipExecutionCallData = ExecutionLib.encodeSingle( + transferOwnershipExecution.target, transferOwnershipExecution.value, transferOwnershipExecution.callData + ); + + vm.prank(dm); + vm.expectRevert("OwnershipTransferEnforcer:invalid-value"); + enforcer.beforeHook(terms_, hex"", singleDefaultMode, transferOwnershipExecutionCallData, bytes32(0), delegator, delegate); + } + // Reverts if the terms length is invalid function test_invalid_termsLength() public { bytes memory invalidTerms = abi.encodePacked(mockContract, uint256(1)); // Too long