chore: value zero validation on non value enforcers

Author: hanzel98

src/enforcers/ERC20PeriodTransferEnforcer.sol

@@ -178,8 +178,9 @@ contract ERC20PeriodTransferEnforcer is CaveatEnforcer {
     )
         private
     {
-        (address target_,, bytes calldata callData_) = _executionCallData.decodeSingle();
+        (address target_, uint256 value_, bytes calldata callData_) = _executionCallData.decodeSingle();
 
+        require(value_ == 0, "ERC20PeriodTransferEnforcer:invalid-value");
         require(callData_.length == 68, "ERC20PeriodTransferEnforcer:invalid-execution-length");
 
         (address token_, uint256 periodAmount_, uint256 periodDuration_, uint256 startDate_) = getTermsInfo(_terms);

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_) =

src/enforcers/ERC20TransferAmountEnforcer.sol

@@ -82,8 +82,9 @@ contract ERC20TransferAmountEnforcer is CaveatEnforcer {
         internal
         returns (uint256 limit_, uint256 spent_)
     {
-        (address target_,, bytes calldata callData_) = _executionCallData.decodeSingle();
+        (address target_, uint256 value_, bytes calldata callData_) = _executionCallData.decodeSingle();
 
+        require(value_ == 0, "ERC20TransferAmountEnforcer:invalid-value");
         require(callData_.length == 68, "ERC20TransferAmountEnforcer:invalid-execution-length");
 
         address allowedContract_;

src/enforcers/ERC721TransferEnforcer.sol

@@ -37,7 +37,8 @@ contract ERC721TransferEnforcer is CaveatEnforcer {
         onlyDefaultExecutionMode(_mode)
     {
         (address permittedContract_, uint256 permittedTokenId_) = getTermsInfo(_terms);
-        (address target_,, bytes calldata callData_) = ExecutionLib.decodeSingle(_executionCallData);
+        (address target_, uint256 value_, bytes calldata callData_) = ExecutionLib.decodeSingle(_executionCallData);
+        require(value_ == 0, "ERC721TransferEnforcer:invalid-value");
 
         // Decode the remaining callData into NFT transfer parameters
         // The calldata should be at least 100 bytes (4 bytes for the selector + 96 bytes for the parameters)

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]);

test/enforcers/ERC20PeriodTransferEnforcer.t.sol

@@ -215,6 +215,15 @@ contract ERC20PeriodTransferEnforcerTest is CaveatEnforcerBaseTest {
         assertEq(availableAfter2, periodAmount - 600);
     }
 
+    /// @notice Reverts if the execution value is not zero.
+    function test_invalidValue() public {
+        bytes memory terms_ = abi.encodePacked(address(basicERC20), periodAmount, periodDuration, startDate);
+        bytes memory callData_ = _encodeERC20Transfer(bob, 100);
+        bytes memory execData_ = _encodeSingleExecution(address(basicERC20), 1 ether, callData_);
+        vm.expectRevert("ERC20PeriodTransferEnforcer:invalid-value");
+        erc20PeriodTransferEnforcer.beforeHook(terms_, "", singleDefaultMode, execData_, dummyDelegationHash, address(0), redeemer);
+    }
+
     // should fail with invalid call type mode (batch instead of single mode)
     function test_revertWithInvalidCallTypeMode() public {
         bytes memory executionCallData_ = ExecutionLib.encodeBatch(new Execution[](2));

test/enforcers/ERC20StreamingEnforcer.t.sol

@@ -172,6 +172,19 @@ contract ERC20StreamingEnforcerTest is CaveatEnforcerBaseTest {
         erc20StreamingEnforcer.beforeHook(terms_, bytes(""), singleDefaultMode, execData_, bytes32(0), address(0), alice);
     }
 
+    /// @notice Reverts if the execution value is not zero.
+    function test_invalidValue() public {
+        uint256 initialAmount_ = 100 ether;
+        uint256 maxAmount_ = 100 ether;
+        uint256 amountPerSecond_ = 1 ether;
+        uint256 startTime_ = block.timestamp;
+        bytes memory terms_ = abi.encodePacked(address(basicERC20), initialAmount_, maxAmount_, amountPerSecond_, startTime_);
+        bytes memory callData_ = _encodeERC20Transfer(bob, 100);
+        bytes memory execData_ = _encodeSingleExecution(address(basicERC20), 1 ether, callData_);
+        vm.expectRevert("ERC20StreamingEnforcer:invalid-value");
+        erc20StreamingEnforcer.beforeHook(terms_, "", singleDefaultMode, execData_, bytes32(0), address(0), alice);
+    }
+
     //////////////////// Valid cases //////////////////////
     /**
      * @notice Test getTermsInfo() on correct 148-byte terms
@@ -418,7 +431,7 @@ contract ERC20StreamingEnforcerTest is CaveatEnforcerBaseTest {
     /**
      * @notice Integration test: Successful native token streaming via delegation.
      * A delegation is created that uses the erc20StreamingEnforcer. Two native token transfers
-     * (user ops) are executed sequentially. The test verifies that the enforcer’s state is updated
+     * (user ops) are executed sequentially. The test verifies that the enforcer's state is updated
      * correctly and that the available amount decreases as expected.
      */
     function test_nativeTokenStreamingIntegration_Success() public {

test/enforcers/ERC20TransferAmountEnforcer.t.sol

@@ -446,6 +446,34 @@ contract ERC20TransferAmountEnforcerTest is CaveatEnforcerBaseTest {
     function _getEnforcer() internal view override returns (ICaveatEnforcer) {
         return ICaveatEnforcer(address(erc20TransferAmountEnforcer));
     }
+
+    /// @notice Reverts if the execution value is not zero.
+    function test_invalidValue() public {
+        uint256 spendingLimit_ = 1 ether;
+        Execution memory execution_ = Execution({
+            target: address(basicERC20),
+            value: 1 ether,
+            callData: abi.encodeWithSelector(IERC20.transfer.selector, address(users.bob.deleGator), spendingLimit_)
+        });
+        bytes memory executionCallData_ = ExecutionLib.encodeSingle(execution_.target, execution_.value, execution_.callData);
+        bytes memory inputTerms_ = abi.encodePacked(address(basicERC20), spendingLimit_);
+        Caveat[] memory caveats_ = new Caveat[](1);
+        caveats_[0] = Caveat({ args: hex"", enforcer: address(erc20TransferAmountEnforcer), terms: inputTerms_ });
+        Delegation memory delegation_ = Delegation({
+            delegate: address(users.bob.deleGator),
+            delegator: address(users.alice.deleGator),
+            authority: ROOT_AUTHORITY,
+            caveats: caveats_,
+            salt: 0,
+            signature: hex""
+        });
+        bytes32 delegationHash_ = EncoderLib._getDelegationHash(delegation_);
+        vm.prank(address(delegationManager));
+        vm.expectRevert("ERC20TransferAmountEnforcer:invalid-value");
+        erc20TransferAmountEnforcer.beforeHook(
+            inputTerms_, hex"", singleDefaultMode, executionCallData_, delegationHash_, address(0), address(0)
+        );
+    }
 }
 
 /// @notice A mock token that allows us to simulate failed transfers.

test/enforcers/ERC721TransferEnforcer.t.sol

@@ -209,6 +209,15 @@ contract ERC721TransferEnforcerTest is CaveatEnforcerBaseTest {
         erc721TransferEnforcer.beforeHook(hex"", hex"", singleTryMode, hex"", bytes32(0), address(0), address(0));
     }
 
+    /// @notice Reverts if the execution value is not zero.
+    function test_invalidValue() public {
+        bytes memory terms_ = abi.encodePacked(address(token), TOKEN_ID);
+        bytes memory callData_ = abi.encodeWithSelector(IERC721.transferFrom.selector, address(this), address(0xBEEF), TOKEN_ID);
+        bytes memory execData_ = ExecutionLib.encodeSingle(address(token), 1 ether, callData_);
+        vm.expectRevert("ERC721TransferEnforcer:invalid-value");
+        erc721TransferEnforcer.beforeHook(terms_, "", singleDefaultMode, execData_, keccak256(""), address(0), address(0));
+    }
+
     ////////////////////// Integration //////////////////////
 
     /// @notice Integration test for valid transfer using transferFrom selector.

test/enforcers/OwnershipTransferEnforcer.t.sol

@@ -138,6 +138,23 @@ contract OwnershipTransferEnforcerTest is CaveatEnforcerBaseTest {
         enforcer.beforeHook(hex"", hex"", singleTryMode, hex"", bytes32(0), address(0), address(0));
     }
 
+    /// @notice Reverts if the execution value is not zero.
+    function test_invalidValue() public {
+        bytes memory terms_ = abi.encodePacked(mockContract);
+        transferOwnershipExecution = Execution({
+            target: mockContract,
+            value: 1 ether,
+            callData: abi.encodeWithSelector(bytes4(keccak256("transferOwnership(address)")), delegate)
+        });
+        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);
+    }
+
     function _getEnforcer() internal view override returns (ICaveatEnforcer) {
         return ICaveatEnforcer(address(enforcer));
     }