From a069cba9d5bcb5a620c8559935356ce32bd34fd9 Mon Sep 17 00:00:00 2001 From: Hanzel Anchia Mena <33629234+hanzel98@users.noreply.github.com> Date: Fri, 6 Sep 2024 15:56:47 -0600 Subject: [PATCH] Audit Mitigation September - 3.1 and 3.2 Payment Enforcer Improvements (#436) --- script/coverage.sh | 2 +- src/enforcers/NativeTokenPaymentEnforcer.sol | 37 +- .../NativeTokenPaymentEnforcer.t.sol | 673 ++++++++++++------ 3 files changed, 471 insertions(+), 241 deletions(-) diff --git a/script/coverage.sh b/script/coverage.sh index a98a53f..f64c049 100755 --- a/script/coverage.sh +++ b/script/coverage.sh @@ -37,4 +37,4 @@ then --output-directory "$folder_path" \ "$folder_path/filtered-lcov.info" open "$folder_path/index.html" -fi +fi \ No newline at end of file diff --git a/src/enforcers/NativeTokenPaymentEnforcer.sol b/src/enforcers/NativeTokenPaymentEnforcer.sol index c6ff002..8023cee 100644 --- a/src/enforcers/NativeTokenPaymentEnforcer.sol +++ b/src/enforcers/NativeTokenPaymentEnforcer.sol @@ -11,11 +11,14 @@ import { IDelegationManager } from "../interfaces/IDelegationManager.sol"; /** * @title NativeTokenPaymentEnforcer * @notice This contract enforces payment in native token (e.g., ETH) for the right to use a delegation. - * @dev The redeemer must include a payment delegation in the arguments when executing an execution. - * The payment, made in native token, is processed during the execution of the delegated execution, ensuring that the - * enforced conditions are met. - * Combining `NativeTokenTransferAmountEnforcer` and `ArgsEqualityCheckEnforcer` when creating the payment delegation is recommended - * to prevent front-running attacks. + * @dev The redeemer must include an allowance delegation when executing the payment. + * The payment, made in native tokens, is processed during the execution of the allowance delegation to ensure that + * the payment conditions set by the terms are met. + * @dev When constructing the leaf allowance delegation, the use of `ArgsEqualityCheckEnforcer` is mandatory placed as the first + * caveat to prevent front-running attacks. This ensures that the execution arguments (delegationHash and redeemer) match exactly, + * protecting the delegation. + * @dev It is recommended to combine the allowance delegation with the `NativeTokenTransferAmountEnforcer` to ensure the correct + * token transfer amount. * @dev Requires the redeemer to be a DeleGator that supports the single execution mode. */ contract NativeTokenPaymentEnforcer is CaveatEnforcer { @@ -53,7 +56,7 @@ contract NativeTokenPaymentEnforcer is CaveatEnforcer { * @notice Enforces the conditions that should hold after a transaction is performed. * @param _terms Encoded 52 packed bytes where: the first 20 bytes are the address of the recipient, * the next 32 bytes are the amount to charge for the delegation. - * @param _args Encoded arguments containing the delegation chain for the payment. + * @param _args Encoded arguments containing the allowance delegation chain for the payment. * @param _delegationHash The hash of the delegation. * @param _delegator The address of the delegator. * @param _redeemer The address that is redeeming the delegation. @@ -75,20 +78,20 @@ contract NativeTokenPaymentEnforcer is CaveatEnforcer { // Decode the payment terms and arguments (address recipient_, uint256 amount_) = getTermsInfo(_terms); - Delegation[] memory delegations_ = abi.decode(_args, (Delegation[])); + Delegation[] memory allowanceDelegations_ = abi.decode(_args, (Delegation[])); - // Assign the delegation hash as the args to the args equality enforcer. - for (uint256 x = 0; x < delegations_.length; ++x) { - Delegation memory delegation_ = delegations_[x]; - for (uint256 i = 0; i < delegation_.caveats.length; ++i) { - if (delegation_.caveats[i].enforcer == argsEqualityCheckEnforcer) { - delegation_.caveats[i].args = abi.encodePacked(_delegationHash); - } - } - } + require(allowanceDelegations_.length > 0, "NativeTokenPaymentEnforcer:invalid-allowance-delegations-length"); + + require( + allowanceDelegations_[0].caveats.length > 0 && allowanceDelegations_[0].caveats[0].enforcer == argsEqualityCheckEnforcer, + "NativeTokenPaymentEnforcer:missing-argsEqualityCheckEnforcer" + ); + + // The Args Enforcer with this data (hash & redeemer) must be the first Enforcer in the payment delegations caveats + allowanceDelegations_[0].caveats[0].args = abi.encodePacked(_delegationHash, _redeemer); bytes[] memory permissionContexts_ = new bytes[](1); - permissionContexts_[0] = abi.encode(delegations_); + permissionContexts_[0] = abi.encode(allowanceDelegations_); bytes[] memory executionCallDatas_ = new bytes[](1); executionCallDatas_[0] = ExecutionLib.encodeSingle(recipient_, amount_, hex""); diff --git a/test/enforcers/NativeTokenPaymentEnforcer.t.sol b/test/enforcers/NativeTokenPaymentEnforcer.t.sol index 94fbb96..755bbea 100644 --- a/test/enforcers/NativeTokenPaymentEnforcer.t.sol +++ b/test/enforcers/NativeTokenPaymentEnforcer.t.sol @@ -26,6 +26,18 @@ contract NativeTokenPaymentEnforcerTest is CaveatEnforcerBaseTest { AllowedTargetsEnforcer public allowedTargetsEnforcer; ArgsEqualityCheckEnforcer public argsEqualityCheckEnforcer; + address public validRedeemer; + address public invalidRedeemer; + address public paymentRecipient; + uint256 public paymentAmount; + bytes public paymentTerms; + Execution public execution; + bytes public executionCalldata; + bytes public allowanceTerms; + bytes public argsEnforcerTerms; + bytes public argsWithBobAllowance; + ModeCode public modeSimpleSingle = ModeLib.encodeSimpleSingle(); + function setUp() public override { super.setUp(); nativeTokenTransferAmountEnforcer = new NativeTokenTransferAmountEnforcer(); @@ -39,11 +51,32 @@ contract NativeTokenPaymentEnforcerTest is CaveatEnforcerBaseTest { vm.label(address(limitedCallsEnforcer), "Limited Calls Enforcer"); allowedTargetsEnforcer = new AllowedTargetsEnforcer(); vm.label(address(allowedTargetsEnforcer), "Allowed Targets Enforcer"); + validRedeemer = address(users.bob.deleGator); + invalidRedeemer = address(users.carol.deleGator); + paymentRecipient = address(users.alice.deleGator); + paymentAmount = 1 ether; + execution = Execution({ + target: address(aliceDeleGatorCounter), + value: 0, + callData: abi.encodeWithSelector(Counter.increment.selector) + }); + + executionCalldata = ExecutionLib.encodeSingle(execution.target, execution.value, execution.callData); + paymentTerms = abi.encodePacked(paymentRecipient, paymentAmount); } //////////////////// Valid cases ////////////////////// - // Should decode the terms + // Should set the initialization values correctly + function test_readInitializationValues() public { + address delegationManager_ = address(9999999999); + address argsEnforcer_ = address(8888888888); + nativeTokenPaymentEnforcer = new NativeTokenPaymentEnforcer(IDelegationManager(delegationManager_), argsEnforcer_); + assertEq(address(nativeTokenPaymentEnforcer.delegationManager()), delegationManager_); + assertEq(nativeTokenPaymentEnforcer.argsEqualityCheckEnforcer(), argsEnforcer_); + } + + // The terms can be decoded with the enforcer function test_decodesTheTerms() public { address recipient_ = address(users.alice.deleGator); uint256 amount_ = 1 ether; @@ -56,48 +89,38 @@ contract NativeTokenPaymentEnforcerTest is CaveatEnforcerBaseTest { // Should SUCCEED if the payment is valid function test_validationPassWithValidPayment() public { - Delegation[] memory paymentDelegations_ = new Delegation[](1); - paymentDelegations_[0] = Delegation({ - delegate: address(nativeTokenPaymentEnforcer), - delegator: address(users.bob.deleGator), - authority: ROOT_AUTHORITY, - caveats: new Caveat[](0), - salt: 0, - signature: hex"" - }); + (bytes32 delegationHash_,) = _getExampleDelegation(paymentTerms, hex""); - paymentDelegations_[0] = signDelegation(users.bob, paymentDelegations_[0]); - - bytes memory args_ = abi.encode(paymentDelegations_); - - address recipient_ = address(users.alice.deleGator); - bytes memory terms_ = abi.encodePacked(recipient_, uint256(1 ether)); - - (bytes32 delegationHash_,) = _getExampleDelegation(terms_, hex""); - - Execution memory execution_ = Execution({ - target: address(aliceDeleGatorCounter), - value: 0, - callData: abi.encodeWithSelector(Counter.increment.selector) - }); - bytes memory executionCallData_ = ExecutionLib.encodeSingle(execution_.target, execution_.value, execution_.callData); - ModeCode mode_ = ModeLib.encodeSimpleSingle(); + (, argsWithBobAllowance) = _getAllowanceDelegation(delegationHash_, address(users.bob.deleGator)); vm.startPrank(address(delegationManager)); vm.expectEmit(true, true, true, true, address(nativeTokenPaymentEnforcer)); emit NativeTokenPaymentEnforcer.ValidatedPayment( - address(delegationManager), delegationHash_, recipient_, address(users.alice.deleGator), address(0), 1 ether + address(delegationManager), + delegationHash_, + paymentRecipient, + address(users.alice.deleGator), + address(users.bob.deleGator), + 1 ether ); nativeTokenPaymentEnforcer.afterHook( - terms_, args_, mode_, executionCallData_, delegationHash_, address(users.alice.deleGator), address(0) + paymentTerms, + argsWithBobAllowance, + modeSimpleSingle, + executionCalldata, + delegationHash_, + address(users.alice.deleGator), + validRedeemer ); } - // Should SUCCEED to make the payment with a redelegation - function test_validationPassWithValidRedelegationPayment() public { - Delegation[] memory paymentDelegations_ = new Delegation[](2); - paymentDelegations_[1] = Delegation({ + // Should SUCCEED to pay with an allowance redelegation + function test_validationPassWithValidRedelegationAllowance() public { + (bytes32 delegationHash_,) = _getExampleDelegation(paymentTerms, hex""); + + Delegation[] memory allowanceDelegations_ = new Delegation[](2); + allowanceDelegations_[1] = Delegation({ delegate: address(users.bob.deleGator), delegator: address(users.carol.deleGator), authority: ROOT_AUTHORITY, @@ -106,132 +129,183 @@ contract NativeTokenPaymentEnforcerTest is CaveatEnforcerBaseTest { signature: hex"" }); - paymentDelegations_[1] = signDelegation(users.carol, paymentDelegations_[1]); + allowanceDelegations_[1] = signDelegation(users.carol, allowanceDelegations_[1]); - paymentDelegations_[0] = Delegation({ + argsEnforcerTerms = abi.encodePacked(delegationHash_, address(users.bob.deleGator)); + Caveat[] memory caveats_ = new Caveat[](1); + caveats_[0] = Caveat({ args: hex"", enforcer: address(argsEqualityCheckEnforcer), terms: argsEnforcerTerms }); + + allowanceDelegations_[0] = Delegation({ delegate: address(nativeTokenPaymentEnforcer), delegator: address(users.bob.deleGator), - authority: EncoderLib._getDelegationHash(paymentDelegations_[1]), - caveats: new Caveat[](0), + authority: EncoderLib._getDelegationHash(allowanceDelegations_[1]), + caveats: caveats_, salt: 0, signature: hex"" }); - paymentDelegations_[0] = signDelegation(users.bob, paymentDelegations_[0]); - - bytes memory args_ = abi.encode(paymentDelegations_); + allowanceDelegations_[0] = signDelegation(users.bob, allowanceDelegations_[0]); - address recipient_ = address(users.alice.deleGator); - bytes memory terms_ = abi.encodePacked(recipient_, uint256(1 ether)); - - (bytes32 delegationHash_,) = _getExampleDelegation(terms_, hex""); - - Execution memory execution_ = Execution({ - target: address(aliceDeleGatorCounter), - value: 0, - callData: abi.encodeWithSelector(Counter.increment.selector) - }); - bytes memory executionCallData_ = ExecutionLib.encodeSingle(execution_.target, execution_.value, execution_.callData); - ModeCode mode_ = ModeLib.encodeSimpleSingle(); + argsWithBobAllowance = abi.encode(allowanceDelegations_); vm.startPrank(address(delegationManager)); vm.expectEmit(true, true, true, true, address(nativeTokenPaymentEnforcer)); emit NativeTokenPaymentEnforcer.ValidatedPayment( - address(delegationManager), delegationHash_, recipient_, address(users.alice.deleGator), address(0), 1 ether + address(delegationManager), + delegationHash_, + paymentRecipient, + address(users.alice.deleGator), + address(users.bob.deleGator), + 1 ether ); nativeTokenPaymentEnforcer.afterHook( - terms_, args_, mode_, executionCallData_, delegationHash_, address(users.alice.deleGator), address(0) + paymentTerms, + argsWithBobAllowance, + modeSimpleSingle, + executionCalldata, + delegationHash_, + address(users.alice.deleGator), + address(users.bob.deleGator) ); } - // Should only overwrite the args of the args equality enforcer + // Should SUCCEED to overwrite only the args on the args equality enforcer function test_onlyOverwriteAllowanceEnforcerArgs() public { - // The terms indicate to send 1 ether to Alice. - bytes memory paymentTerms_ = abi.encodePacked(address(users.alice.deleGator), uint256(1 ether)); - (bytes32 delegationHash_, Delegation memory paidDelegation_) = _getExampleDelegation(paymentTerms_, hex""); - - Delegation[] memory paidDelegations_ = new Delegation[](1); - paidDelegations_[0] = paidDelegation_; + (bytes32 delegationHash_,) = _getExampleDelegation(paymentTerms, hex""); // Create the Allowance enforcer - uint256 allowance_ = 1 ether; - bytes memory allowanceTerms_ = abi.encode(allowance_); - bytes memory argsTerms_ = abi.encode(delegationHash_); + allowanceTerms = abi.encode(paymentAmount); + argsEnforcerTerms = abi.encodePacked(delegationHash_, address(users.bob.deleGator)); - // The args of the nativeTokenTransferAmountEnforcer will ovewriten + // The args of the nativeTokenTransferAmountEnforcer will be overwritten // The limitedCallsEnforcer and allowedTargetsEnforcer should stay the same - Caveat[] memory paymentCaveats_ = new Caveat[](4); - paymentCaveats_[0] = Caveat({ args: hex"", enforcer: address(nativeTokenTransferAmountEnforcer), terms: allowanceTerms_ }); - paymentCaveats_[1] = Caveat({ args: hex"", enforcer: address(limitedCallsEnforcer), terms: abi.encodePacked(uint256(10)) }); - paymentCaveats_[2] = Caveat({ + Caveat[] memory allowanceCaveats_ = new Caveat[](4); + allowanceCaveats_[0] = Caveat({ args: hex"", enforcer: address(argsEqualityCheckEnforcer), terms: argsEnforcerTerms }); + allowanceCaveats_[1] = Caveat({ args: hex"", enforcer: address(nativeTokenTransferAmountEnforcer), terms: allowanceTerms }); + allowanceCaveats_[2] = + Caveat({ args: hex"", enforcer: address(limitedCallsEnforcer), terms: abi.encodePacked(uint256(10)) }); + allowanceCaveats_[3] = Caveat({ args: hex"", enforcer: address(allowedTargetsEnforcer), terms: abi.encodePacked(address(users.alice.deleGator)) }); - paymentCaveats_[3] = Caveat({ args: hex"", enforcer: address(argsEqualityCheckEnforcer), terms: argsTerms_ }); - // Create payment delegation from Bob to NativeTokenPaymentEnforcer - Delegation[] memory paymentDelegations_ = new Delegation[](1); - paymentDelegations_[0] = Delegation({ + // Create allowance delegation from Bob to NativeTokenPaymentEnforcer + Delegation[] memory allowanceDelegations_ = new Delegation[](1); + allowanceDelegations_[0] = Delegation({ delegate: address(nativeTokenPaymentEnforcer), delegator: address(users.bob.deleGator), authority: ROOT_AUTHORITY, - caveats: paymentCaveats_, + caveats: allowanceCaveats_, salt: 0, signature: hex"" }); - paymentDelegations_[0] = signDelegation(users.bob, paymentDelegations_[0]); + allowanceDelegations_[0] = signDelegation(users.bob, allowanceDelegations_[0]); - // The args contain the payment delegation to redeem - bytes memory args_ = abi.encode(paymentDelegations_); - - Execution memory execution_ = Execution({ - target: address(aliceDeleGatorCounter), - value: 0, - callData: abi.encodeWithSelector(Counter.increment.selector) - }); + // The args contain the allowance delegation to redeem + argsWithBobAllowance = abi.encode(allowanceDelegations_); vm.startPrank(address(delegationManager)); // Expect this to be overwritten in the event - paymentDelegations_[0].caveats[3].args = abi.encodePacked(delegationHash_); + allowanceDelegations_[0].caveats[0].args = argsEnforcerTerms; // Checks the args of the caveats in the DelegationManager event vm.expectEmit(true, true, true, true, address(delegationManager)); emit IDelegationManager.RedeemedDelegation( - paymentDelegations_[0].delegator, address(nativeTokenPaymentEnforcer), paymentDelegations_[0] + allowanceDelegations_[0].delegator, address(nativeTokenPaymentEnforcer), allowanceDelegations_[0] ); nativeTokenPaymentEnforcer.afterHook( - paymentTerms_, - args_, + paymentTerms, + argsWithBobAllowance, ModeLib.encodeSimpleSingle(), - ExecutionLib.encodeSingle(execution_.target, execution_.value, execution_.callData), + executionCalldata, delegationHash_, address(users.alice.deleGator), - address(0) + address(users.bob.deleGator) ); } - ////////////////////// Invalid cases ////////////////////// + // Should FAIL to process the payment if the args equality check enforcer is missing + function test_paymentFailsIfArgsEnforcerIsMissing() public { + (bytes32 delegationHash_,) = _getExampleDelegation(paymentTerms, hex""); - // should FAIL to get terms info when passing an invalid terms length - function test_getTermsInfoFailsForInvalidLength() public { - vm.expectRevert("NativeTokenPaymentEnforcer:invalid-terms-length"); - nativeTokenPaymentEnforcer.getTermsInfo(bytes("1")); + // Create the Allowance enforcer + allowanceTerms = abi.encode(paymentAmount); + + // Even with other enforcers it should revert if it does not include the args enforcer + Caveat[] memory allowanceCaveats_ = new Caveat[](3); + allowanceCaveats_[0] = Caveat({ args: hex"", enforcer: address(nativeTokenTransferAmountEnforcer), terms: allowanceTerms }); + allowanceCaveats_[1] = + Caveat({ args: hex"", enforcer: address(limitedCallsEnforcer), terms: abi.encodePacked(uint256(10)) }); + allowanceCaveats_[2] = Caveat({ + args: hex"", + enforcer: address(allowedTargetsEnforcer), + terms: abi.encodePacked(address(users.alice.deleGator)) + }); + + // Create allowance delegation from Bob to NativeTokenPaymentEnforcer + Delegation[] memory allowanceDelegations_ = new Delegation[](1); + allowanceDelegations_[0] = Delegation({ + delegate: address(nativeTokenPaymentEnforcer), + delegator: address(users.bob.deleGator), + authority: ROOT_AUTHORITY, + caveats: allowanceCaveats_, + salt: 0, + signature: hex"" + }); + + allowanceDelegations_[0] = signDelegation(users.bob, allowanceDelegations_[0]); + + // The args contain the allowance delegation to redeem + argsWithBobAllowance = abi.encode(allowanceDelegations_); + + vm.startPrank(address(delegationManager)); + + vm.expectRevert("NativeTokenPaymentEnforcer:missing-argsEqualityCheckEnforcer"); + nativeTokenPaymentEnforcer.afterHook( + paymentTerms, + argsWithBobAllowance, + ModeLib.encodeSimpleSingle(), + executionCalldata, + delegationHash_, + address(users.alice.deleGator), + address(users.bob.deleGator) + ); } - // Should FAIL if the payment is insufficient - function test_validationFailWithInsufficientPayment() public { - address mockDelegationManager_ = address(new MockDelegationManager()); - // Overriding the delegation manager for testing purposes - nativeTokenPaymentEnforcer = - new NativeTokenPaymentEnforcer(IDelegationManager(mockDelegationManager_), address(nativeTokenTransferAmountEnforcer)); - vm.label(address(nativeTokenPaymentEnforcer), "Native Paid Enforcer"); + // Should FAIL if the allowance delegation is empty + function test_paymentFailsIfAllowanceDelegationIsEmpty() public { + (bytes32 delegationHash_,) = _getExampleDelegation(paymentTerms, hex""); + + // This is empty to make it revert + Delegation[] memory allowanceDelegations_ = new Delegation[](0); + + // The args contain the allowance delegation to redeem + argsWithBobAllowance = abi.encode(allowanceDelegations_); - Delegation[] memory paymentDelegations_ = new Delegation[](1); - paymentDelegations_[0] = Delegation({ + vm.startPrank(address(delegationManager)); + + vm.expectRevert("NativeTokenPaymentEnforcer:invalid-allowance-delegations-length"); + nativeTokenPaymentEnforcer.afterHook( + paymentTerms, + argsWithBobAllowance, + ModeLib.encodeSimpleSingle(), + executionCalldata, + delegationHash_, + address(users.alice.deleGator), + address(users.bob.deleGator) + ); + } + + // Should FAIL if the allowance delegation caveats are empty + function test_validationFailWithEmptyCaveatsInAllowanceDelegation() public { + (bytes32 delegationHash_,) = _getExampleDelegation(paymentTerms, hex""); + + Delegation[] memory allowanceDelegations_ = new Delegation[](1); + allowanceDelegations_[0] = Delegation({ delegate: address(nativeTokenPaymentEnforcer), delegator: address(users.bob.deleGator), authority: ROOT_AUTHORITY, @@ -239,27 +313,88 @@ contract NativeTokenPaymentEnforcerTest is CaveatEnforcerBaseTest { salt: 0, signature: hex"" }); - paymentDelegations_[0] = signDelegation(users.bob, paymentDelegations_[0]); - // Using a mock delegation manager - bytes memory args_ = abi.encode(paymentDelegations_); + allowanceDelegations_[0] = signDelegation(users.bob, allowanceDelegations_[0]); + argsWithBobAllowance = abi.encode(allowanceDelegations_); - address recipient_ = address(users.alice.deleGator); - bytes memory terms_ = abi.encodePacked(recipient_, uint256(1 ether)); + vm.startPrank(address(delegationManager)); + vm.expectRevert("NativeTokenPaymentEnforcer:missing-argsEqualityCheckEnforcer"); + nativeTokenPaymentEnforcer.afterHook( + paymentTerms, + argsWithBobAllowance, + modeSimpleSingle, + executionCalldata, + delegationHash_, + address(0), + address(users.bob.deleGator) + ); + } - (bytes32 delegationHash_,) = _getExampleDelegation(terms_, hex""); + // Should FAIL if the args enforcer is not in the first caveat of the allowance delegation + function test_validationFailIfArgsEnforcerNotInFirstPlace() public { + (bytes32 delegationHash_,) = _getExampleDelegation(paymentTerms, hex""); - Execution memory execution_ = Execution({ - target: address(aliceDeleGatorCounter), - value: 0, - callData: abi.encodeWithSelector(Counter.increment.selector) + Caveat[] memory caveats_ = new Caveat[](2); + allowanceTerms = abi.encode(paymentAmount); + argsEnforcerTerms = abi.encodePacked(delegationHash_, address(users.bob.deleGator)); + caveats_[0] = Caveat({ args: hex"", enforcer: address(nativeTokenTransferAmountEnforcer), terms: allowanceTerms }); + caveats_[1] = Caveat({ args: hex"", enforcer: address(argsEqualityCheckEnforcer), terms: argsEnforcerTerms }); + + Delegation[] memory allowanceDelegations_ = new Delegation[](1); + allowanceDelegations_[0] = Delegation({ + delegate: address(nativeTokenPaymentEnforcer), + delegator: address(users.bob.deleGator), + authority: ROOT_AUTHORITY, + caveats: caveats_, + salt: 0, + signature: hex"" }); - bytes memory executionCallData_ = ExecutionLib.encodeSingle(execution_.target, execution_.value, execution_.callData); - ModeCode mode_ = ModeLib.encodeSimpleSingle(); + + allowanceDelegations_[0] = signDelegation(users.bob, allowanceDelegations_[0]); + argsWithBobAllowance = abi.encode(allowanceDelegations_); + + vm.startPrank(address(delegationManager)); + vm.expectRevert("NativeTokenPaymentEnforcer:missing-argsEqualityCheckEnforcer"); + nativeTokenPaymentEnforcer.afterHook( + paymentTerms, + argsWithBobAllowance, + modeSimpleSingle, + executionCalldata, + delegationHash_, + address(0), + address(users.bob.deleGator) + ); + } + + // should FAIL to get terms info when passing invalid terms length + function test_getTermsInfoFailsForInvalidLength() public { + vm.expectRevert("NativeTokenPaymentEnforcer:invalid-terms-length"); + nativeTokenPaymentEnforcer.getTermsInfo(bytes("1")); + } + + // Should FAIL if the payment is insufficient + function test_validationFailWithInsufficientPayment() public { + address mockDelegationManager_ = address(new MockDelegationManager()); + // Overwriting the delegation manager for testing purposes + nativeTokenPaymentEnforcer = + new NativeTokenPaymentEnforcer(IDelegationManager(mockDelegationManager_), address(argsEqualityCheckEnforcer)); + vm.label(address(nativeTokenPaymentEnforcer), "Native Paid Enforcer"); + + (bytes32 delegationHash_,) = _getExampleDelegation(paymentTerms, hex""); + + (, argsWithBobAllowance) = _getAllowanceDelegation(delegationHash_, address(users.bob.deleGator)); vm.startPrank(mockDelegationManager_); vm.expectRevert("NativeTokenPaymentEnforcer:payment-not-received"); - nativeTokenPaymentEnforcer.afterHook(terms_, args_, mode_, executionCallData_, delegationHash_, address(0), address(0)); + nativeTokenPaymentEnforcer.afterHook( + paymentTerms, + argsWithBobAllowance, + modeSimpleSingle, + executionCalldata, + delegationHash_, + address(0), + address(users.bob.deleGator) + ); } // Should FAIL if the sender is different from the delegation manager. @@ -272,27 +407,25 @@ contract NativeTokenPaymentEnforcerTest is CaveatEnforcerBaseTest { ); } + // Should SUCCEED to charge the payment from the allowance delegation function test_chargePaymentFromAllowance() public { // The terms indicate to send 1 ether to Alice. - address recipient_ = address(users.alice.deleGator); - bytes memory paymentTerms_ = abi.encodePacked(recipient_, uint256(1 ether)); - (bytes32 delegationHash_, Delegation memory paidDelegation_) = _getExampleDelegation(paymentTerms_, hex""); + (bytes32 delegationHash_, Delegation memory paidDelegation_) = _getExampleDelegation(paymentTerms, hex""); Delegation[] memory paidDelegations_ = new Delegation[](1); paidDelegations_[0] = paidDelegation_; // Create the Allowance enforcer - uint256 allowance_ = 1 ether; - bytes memory allowanceTerms_ = abi.encode(allowance_); - bytes memory argsTerms_ = abi.encode(delegationHash_); + allowanceTerms = abi.encode(paymentAmount); + argsEnforcerTerms = abi.encodePacked(delegationHash_, address(users.bob.deleGator)); Caveat[] memory caveats_ = new Caveat[](2); - caveats_[0] = Caveat({ args: hex"", enforcer: address(nativeTokenTransferAmountEnforcer), terms: allowanceTerms_ }); - caveats_[1] = Caveat({ args: hex"", enforcer: address(argsEqualityCheckEnforcer), terms: argsTerms_ }); + caveats_[0] = Caveat({ args: hex"", enforcer: address(argsEqualityCheckEnforcer), terms: argsEnforcerTerms }); + caveats_[1] = Caveat({ args: hex"", enforcer: address(nativeTokenTransferAmountEnforcer), terms: allowanceTerms }); - // Create payment delegation from Bob to NativeTokenPaymentEnforcer - Delegation[] memory paymentDelegations_ = new Delegation[](1); - paymentDelegations_[0] = Delegation({ + // Create allowance delegation from Bob to NativeTokenPaymentEnforcer + Delegation[] memory allowanceDelegations_ = new Delegation[](1); + allowanceDelegations_[0] = Delegation({ delegate: address(nativeTokenPaymentEnforcer), delegator: address(users.bob.deleGator), authority: ROOT_AUTHORITY, @@ -301,24 +434,18 @@ contract NativeTokenPaymentEnforcerTest is CaveatEnforcerBaseTest { signature: hex"" }); - paymentDelegations_[0] = signDelegation(users.bob, paymentDelegations_[0]); + allowanceDelegations_[0] = signDelegation(users.bob, allowanceDelegations_[0]); - // The args contain the payment delegation to redeem - bytes memory args_ = abi.encode(paymentDelegations_); - - Execution memory execution_ = Execution({ - target: address(aliceDeleGatorCounter), - value: 0, - callData: abi.encodeWithSelector(Counter.increment.selector) - }); + // The args contain the allowance delegation to redeem + argsWithBobAllowance = abi.encode(allowanceDelegations_); uint256 aliceBalanceBefore_ = address(users.alice.deleGator).balance; assertEq(aliceDeleGatorCounter.count(), 0); - // Pass the delegation payment in the args. - paidDelegations_[0].caveats[0].args = args_; - invokeDelegation_UserOp(users.bob, paidDelegations_, execution_); + // Pass the delegation allowance in the args. + paidDelegations_[0].caveats[0].args = argsWithBobAllowance; + invokeDelegation_UserOp(users.bob, paidDelegations_, execution); assertEq(aliceDeleGatorCounter.count(), 1); @@ -326,100 +453,157 @@ contract NativeTokenPaymentEnforcerTest is CaveatEnforcerBaseTest { assertEq(address(users.alice.deleGator).balance, aliceBalanceBefore_ + 1 ether); } - function test_delegationFrontRunning() public { - address recipient_ = address(users.alice.deleGator); - // The original terms indicating to send 1 ether to Alice as the payment for Bob - bytes memory originalPaymentTerms_ = abi.encodePacked(recipient_, uint256(1 ether)); - - (, Delegation memory originalPaidDelegation_) = _getExampleDelegation(originalPaymentTerms_, hex""); - Delegation[] memory originalPaidDelegations_ = new Delegation[](1); - originalPaidDelegations_[0] = originalPaidDelegation_; - - // The malicious terms indicating to send 1 ether to Alice as the payment for Carol - bytes memory maliciousPaymentTerms_ = abi.encodePacked(recipient_, uint256(1 ether)); - (, Delegation memory maliciousPaidDelegation_) = _getMaliciousDelegation(maliciousPaymentTerms_, hex""); - Delegation[] memory maliciousPaidDelegations_ = new Delegation[](1); - maliciousPaidDelegations_[0] = maliciousPaidDelegation_; - - // Create the Allowance enforcer (No args enforcer comparison, prone to front-running) - uint256 allowance_ = 1 ether; - bytes memory allowanceTerms_ = abi.encode(allowance_); - Caveat[] memory caveats_ = new Caveat[](1); - caveats_[0] = Caveat({ args: hex"", enforcer: address(nativeTokenTransferAmountEnforcer), terms: allowanceTerms_ }); + // Should SUCCEED to redelegate, adding more required payments on each hop. + function test_allowsRedelegationAddingExtraCosts() public { + // Creating paid delegation + Caveat[] memory caveatsAlice_ = new Caveat[](1); + caveatsAlice_[0] = Caveat({ args: hex"", enforcer: address(nativeTokenPaymentEnforcer), terms: paymentTerms }); + Caveat[] memory caveatsBob_ = new Caveat[](1); + caveatsBob_[0] = Caveat({ + args: hex"", + enforcer: address(nativeTokenPaymentEnforcer), + terms: abi.encodePacked(address(users.bob.deleGator), paymentAmount / 2) + }); - // Create payment delegation from Bob to NativeTokenPaymentEnforcer - // This delegation is public any one could front-running it - // Even though Bob created this delegation to pay for his Alice delegation, Carol uses this delegation to pay for her. - Delegation[] memory paymentDelegations_ = new Delegation[](1); - paymentDelegations_[0] = Delegation({ - delegate: address(nativeTokenPaymentEnforcer), - delegator: address(users.bob.deleGator), + Delegation[] memory paidDelegations_ = new Delegation[](2); + paidDelegations_[1] = Delegation({ + delegate: address(users.bob.deleGator), + delegator: address(users.alice.deleGator), authority: ROOT_AUTHORITY, - caveats: caveats_, + caveats: caveatsAlice_, salt: 0, signature: hex"" }); - paymentDelegations_[0] = signDelegation(users.bob, paymentDelegations_[0]); + paidDelegations_[1] = signDelegation(users.alice, paidDelegations_[1]); + bytes32 delegationHashAlice_ = EncoderLib._getDelegationHash(paidDelegations_[1]); - // The args contain the payment delegation to redeem - bytes memory argsWithBobPayment_ = abi.encode(paymentDelegations_); + paidDelegations_[0] = Delegation({ + delegate: address(users.carol.deleGator), + delegator: address(users.bob.deleGator), + authority: delegationHashAlice_, + caveats: caveatsBob_, + salt: 0, + signature: hex"" + }); + paidDelegations_[0] = signDelegation(users.bob, paidDelegations_[0]); + bytes32 delegationHashBob_ = EncoderLib._getDelegationHash(paidDelegations_[0]); - Execution memory execution_ = Execution({ - target: address(aliceDeleGatorCounter), - value: 0, - callData: abi.encodeWithSelector(Counter.increment.selector) + // Creating allowance delegation + Caveat[] memory caveatsToAlice_ = new Caveat[](2); + caveatsToAlice_[0] = Caveat({ + args: hex"", + enforcer: address(argsEqualityCheckEnforcer), + terms: abi.encodePacked(delegationHashAlice_, address(users.carol.deleGator)) }); + caveatsToAlice_[1] = + Caveat({ args: hex"", enforcer: address(nativeTokenTransferAmountEnforcer), terms: abi.encode(paymentAmount) }); - uint256 aliceBalanceBefore_ = address(users.alice.deleGator).balance; + Caveat[] memory caveatsToBob_ = new Caveat[](2); + caveatsToBob_[0] = Caveat({ + args: hex"", + enforcer: address(argsEqualityCheckEnforcer), + terms: abi.encodePacked(delegationHashBob_, address(users.carol.deleGator)) + }); + caveatsToBob_[1] = + Caveat({ args: hex"", enforcer: address(nativeTokenTransferAmountEnforcer), terms: abi.encode(paymentAmount / 2) }); + + // Create allowance delegation from Bob to NativeTokenPaymentEnforcer + Delegation[] memory allowanceDelegationsToAlice_ = new Delegation[](1); + allowanceDelegationsToAlice_[0] = Delegation({ + delegate: address(nativeTokenPaymentEnforcer), + delegator: address(users.carol.deleGator), + authority: ROOT_AUTHORITY, + caveats: caveatsToAlice_, + salt: 0, + signature: hex"" + }); + allowanceDelegationsToAlice_[0] = signDelegation(users.carol, allowanceDelegationsToAlice_[0]); + + Delegation[] memory allowanceDelegationsToBob_ = new Delegation[](1); + allowanceDelegationsToBob_[0] = Delegation({ + delegate: address(nativeTokenPaymentEnforcer), + delegator: address(users.carol.deleGator), + authority: ROOT_AUTHORITY, + caveats: caveatsToBob_, + salt: 0, + signature: hex"" + }); + allowanceDelegationsToBob_[0] = signDelegation(users.carol, allowanceDelegationsToBob_[0]); + // Check state before execution + uint256 aliceBalanceBefore_ = address(users.alice.deleGator).balance; + uint256 bobBalanceBefore_ = address(users.bob.deleGator).balance; + uint256 carolBalanceBefore_ = address(users.carol.deleGator).balance; assertEq(aliceDeleGatorCounter.count(), 0); - // Pass the delegation payment in the args. - maliciousPaidDelegations_[0].caveats[0].args = argsWithBobPayment_; - invokeDelegation_UserOp(users.carol, maliciousPaidDelegations_, execution_); + // Pass the allowances in the args. + paidDelegations_[1].caveats[0].args = abi.encode(allowanceDelegationsToAlice_); + paidDelegations_[0].caveats[0].args = abi.encode(allowanceDelegationsToBob_); + invokeDelegation_UserOp(users.carol, paidDelegations_, execution); + assertEq(aliceDeleGatorCounter.count(), 1); - // Alice received the payment + // Alice and Bob received the payment, the funds were taken from Carol assertEq(address(users.alice.deleGator).balance, aliceBalanceBefore_ + 1 ether); - - // Pass the delegation payment in the args. - originalPaidDelegations_[0].caveats[0].args = argsWithBobPayment_; - invokeDelegation_UserOp(users.bob, originalPaidDelegations_, execution_); - // The execution did not work because the allowance has already been used. - assertEq(aliceDeleGatorCounter.count(), 1); + assertEq(address(users.bob.deleGator).balance, bobBalanceBefore_ + paymentAmount / 2); + assertTrue(address(users.carol.deleGator).balance < (carolBalanceBefore_ - paymentAmount - paymentAmount / 2)); + assertApproxEqAbs(address(users.carol.deleGator).balance, (carolBalanceBefore_ - paymentAmount - paymentAmount / 2), 1e8); } - function test_delegationArgsEnforcerPreventFrontRunning() public { - // The original terms indicating to send 1 ether to Alice as the payment for Bob - address recipient_ = address(users.alice.deleGator); - bytes memory originalPaymentTerms_ = abi.encodePacked(recipient_, uint256(1 ether)); + // Should SUCCEED to prevent front running using the args enforcer, catches the error + function test_delegationPreventFrontRunningWithArgsCatchError() public { + (bytes32 originalDelegationHash_, Delegation memory originalPaidDelegation_) = _getExampleDelegation(paymentTerms, hex""); - (bytes32 originalPaidDelegationHash_, Delegation memory originalPaidDelegation_) = - _getExampleDelegation(originalPaymentTerms_, hex""); - Delegation[] memory originalPaidDelegations_ = new Delegation[](1); - originalPaidDelegations_[0] = originalPaidDelegation_; + (bytes32 maliciousDelegationHash_, Delegation memory maliciousPaidDelegation_) = + _getMaliciousDelegation(paymentTerms, hex""); - // The malicious terms indicating to send 1 ether to Alice as the payment for Carol - bytes memory maliciousPaymentTerms_ = abi.encodePacked(recipient_, uint256(1 ether)); + (, argsWithBobAllowance) = _getAllowanceDelegation(originalDelegationHash_, address(users.bob.deleGator)); + + vm.startPrank(address(delegationManager)); + + // The redeemer is Carol and not Bob who is the delegator of the allowance delegation + vm.expectRevert("ArgsEqualityCheckEnforcer:different-args-and-terms"); + nativeTokenPaymentEnforcer.afterHook( + paymentTerms, + argsWithBobAllowance, + ModeLib.encodeSimpleSingle(), + executionCalldata, + maliciousDelegationHash_, + maliciousPaidDelegation_.delegator, + maliciousPaidDelegation_.delegate + ); + + // The redeemer is Bob who is the delegator of the payment delegations + nativeTokenPaymentEnforcer.afterHook( + paymentTerms, + argsWithBobAllowance, + ModeLib.encodeSimpleSingle(), + executionCalldata, + originalDelegationHash_, + originalPaidDelegation_.delegator, + originalPaidDelegation_.delegate + ); + } - (, Delegation memory maliciousPaidDelegation_) = _getMaliciousDelegation(maliciousPaymentTerms_, hex""); - Delegation[] memory maliciousPaidDelegations_ = new Delegation[](1); - maliciousPaidDelegations_[0] = maliciousPaidDelegation_; + // Should SUCCEED to prevent front running using the args enforcer + function test_delegationPreventFrontRunningWithArgs() public { + (bytes32 openPaidDelegationHash_, Delegation memory openPaidDelegation_) = _getOpenDelegation(paymentTerms, hex""); + Delegation[] memory openPaidDelegations_ = new Delegation[](1); + openPaidDelegations_[0] = openPaidDelegation_; // Create the Allowance enforcer (Combined with args enforcer, prevent front-running) - uint256 allowance_ = 1 ether; - bytes memory allowanceTerms_ = abi.encode(allowance_); - bytes memory argsTerms_ = abi.encode(originalPaidDelegationHash_); + allowanceTerms = abi.encode(paymentAmount); + argsEnforcerTerms = abi.encodePacked(openPaidDelegationHash_, address(users.bob.deleGator)); Caveat[] memory caveats_ = new Caveat[](2); - caveats_[0] = Caveat({ args: hex"", enforcer: address(nativeTokenTransferAmountEnforcer), terms: allowanceTerms_ }); - caveats_[1] = Caveat({ args: hex"", enforcer: address(argsEqualityCheckEnforcer), terms: argsTerms_ }); + caveats_[0] = Caveat({ args: hex"", enforcer: address(argsEqualityCheckEnforcer), terms: argsEnforcerTerms }); + caveats_[1] = Caveat({ args: hex"", enforcer: address(nativeTokenTransferAmountEnforcer), terms: allowanceTerms }); - // Create payment delegation from Bob to NativeTokenPaymentEnforcer - // This delegation is public any one could front-running it but it is protected with the args enforcer + // Create allowance delegation from Bob to NativeTokenPaymentEnforcer + // This delegation is protected with the args enforcer // Even though Bob created this delegation to pay for his Alice delegation, Carol tries to use it to pay for her. - Delegation[] memory paymentDelegations_ = new Delegation[](1); - paymentDelegations_[0] = Delegation({ + Delegation[] memory allowanceDelegations_ = new Delegation[](1); + allowanceDelegations_[0] = Delegation({ delegate: address(nativeTokenPaymentEnforcer), delegator: address(users.bob.deleGator), authority: ROOT_AUTHORITY, @@ -427,33 +611,26 @@ contract NativeTokenPaymentEnforcerTest is CaveatEnforcerBaseTest { salt: 0, signature: hex"" }); - paymentDelegations_[0] = signDelegation(users.bob, paymentDelegations_[0]); + allowanceDelegations_[0] = signDelegation(users.bob, allowanceDelegations_[0]); - // The args contain the payment delegation to redeem - bytes memory argsWithBobPayment_ = abi.encode(paymentDelegations_); - - Execution memory execution_ = Execution({ - target: address(aliceDeleGatorCounter), - value: 0, - callData: abi.encodeWithSelector(Counter.increment.selector) - }); + // The args contain the allowance delegation to redeem + argsWithBobAllowance = abi.encode(allowanceDelegations_); uint256 aliceBalanceBefore_ = address(users.alice.deleGator).balance; assertEq(aliceDeleGatorCounter.count(), 0); - // Pass the delegation payment in the args. - maliciousPaidDelegations_[0].caveats[0].args = argsWithBobPayment_; - invokeDelegation_UserOp(users.carol, maliciousPaidDelegations_, execution_); - // The execution did not work because the allowance fails due to the invalid args. + // Pass the delegation allowance in the args. + openPaidDelegations_[0].caveats[0].args = argsWithBobAllowance; + + invokeDelegation_UserOp(users.carol, openPaidDelegations_, execution); + // The execution did not work because the redeemer is not Carol. assertEq(aliceDeleGatorCounter.count(), 0); // Alice did not receive the payment assertEq(address(users.alice.deleGator).balance, aliceBalanceBefore_); - // Pass the delegation payment in the args. - originalPaidDelegations_[0].caveats[0].args = argsWithBobPayment_; - invokeDelegation_UserOp(users.bob, originalPaidDelegations_, execution_); - // The execution works well with a proper args + invokeDelegation_UserOp(users.bob, openPaidDelegations_, execution); + // The execution works well with a proper args and using Bob as redeemer assertEq(aliceDeleGatorCounter.count(), 1); // Alice received the payment assertEq(address(users.alice.deleGator).balance, aliceBalanceBefore_ + 1 ether); @@ -482,6 +659,29 @@ contract NativeTokenPaymentEnforcerTest is CaveatEnforcerBaseTest { delegationHash_ = EncoderLib._getDelegationHash(delegation_); } + function _getOpenDelegation( + bytes memory inputTerms_, + bytes memory args_ + ) + internal + view + returns (bytes32 delegationHash_, Delegation memory delegation_) + { + Caveat[] memory caveats_ = new Caveat[](1); + caveats_[0] = Caveat({ args: args_, enforcer: address(nativeTokenPaymentEnforcer), terms: inputTerms_ }); + + delegation_ = Delegation({ + delegate: delegationManager.ANY_DELEGATE(), + delegator: address(users.alice.deleGator), + authority: ROOT_AUTHORITY, + caveats: caveats_, + salt: 0, + signature: hex"" + }); + delegation_ = signDelegation(users.alice, delegation_); + delegationHash_ = EncoderLib._getDelegationHash(delegation_); + } + function _getMaliciousDelegation( bytes memory inputTerms_, bytes memory args_ @@ -505,6 +705,33 @@ contract NativeTokenPaymentEnforcerTest is CaveatEnforcerBaseTest { delegationHash_ = EncoderLib._getDelegationHash(delegation_); } + function _getAllowanceDelegation( + bytes32 _delegationHash, + address _redeemer + ) + internal + returns (Delegation[] memory allowanceDelegations_, bytes memory encodedallowanceDelegations_) + { + Caveat[] memory caveats_ = new Caveat[](2); + allowanceTerms = abi.encode(paymentAmount); + argsEnforcerTerms = abi.encodePacked(_delegationHash, _redeemer); + caveats_[0] = Caveat({ args: hex"", enforcer: address(argsEqualityCheckEnforcer), terms: argsEnforcerTerms }); + caveats_[1] = Caveat({ args: hex"", enforcer: address(nativeTokenTransferAmountEnforcer), terms: allowanceTerms }); + + allowanceDelegations_ = new Delegation[](1); + allowanceDelegations_[0] = Delegation({ + delegate: address(nativeTokenPaymentEnforcer), + delegator: address(users.bob.deleGator), + authority: ROOT_AUTHORITY, + caveats: caveats_, + salt: 0, + signature: hex"" + }); + + allowanceDelegations_[0] = signDelegation(users.bob, allowanceDelegations_[0]); + encodedallowanceDelegations_ = abi.encode(allowanceDelegations_); + } + function _getEnforcer() internal view override returns (ICaveatEnforcer) { return ICaveatEnforcer(address(nativeTokenPaymentEnforcer)); }