diff --git a/src/enforcers/ERC20PeriodTransferEnforcer.sol b/src/enforcers/ERC20PeriodTransferEnforcer.sol index 01615ed2..16012686 100644 --- a/src/enforcers/ERC20PeriodTransferEnforcer.sol +++ b/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); 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/ERC20TransferAmountEnforcer.sol b/src/enforcers/ERC20TransferAmountEnforcer.sol index 58d79748..716b4c13 100644 --- a/src/enforcers/ERC20TransferAmountEnforcer.sol +++ b/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_; diff --git a/src/enforcers/ERC721TransferEnforcer.sol b/src/enforcers/ERC721TransferEnforcer.sol index 1bd086e3..d433f535 100644 --- a/src/enforcers/ERC721TransferEnforcer.sol +++ b/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) 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/ERC20PeriodTransferEnforcer.t.sol b/test/enforcers/ERC20PeriodTransferEnforcer.t.sol index e8990db9..28ed4520 100644 --- a/test/enforcers/ERC20PeriodTransferEnforcer.t.sol +++ b/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)); diff --git a/test/enforcers/ERC20StreamingEnforcer.t.sol b/test/enforcers/ERC20StreamingEnforcer.t.sol index e4b7b2cf..b299b3bb 100644 --- a/test/enforcers/ERC20StreamingEnforcer.t.sol +++ b/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 { diff --git a/test/enforcers/ERC20TransferAmountEnforcer.t.sol b/test/enforcers/ERC20TransferAmountEnforcer.t.sol index ad437049..ce884935 100644 --- a/test/enforcers/ERC20TransferAmountEnforcer.t.sol +++ b/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. diff --git a/test/enforcers/ERC721TransferEnforcer.t.sol b/test/enforcers/ERC721TransferEnforcer.t.sol index 387f4d25..89cbd6cb 100644 --- a/test/enforcers/ERC721TransferEnforcer.t.sol +++ b/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. diff --git a/test/enforcers/OwnershipTransferEnforcer.t.sol b/test/enforcers/OwnershipTransferEnforcer.t.sol index 26de10f2..7734ccdf 100644 --- a/test/enforcers/OwnershipTransferEnforcer.t.sol +++ b/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)); }