diff --git a/audits/cyfrin/cyfrin-5-26.pdf b/audits/cyfrin/cyfrin-5-26.pdf new file mode 100644 index 00000000..006668ca Binary files /dev/null and b/audits/cyfrin/cyfrin-5-26.pdf differ diff --git a/documents/CaveatEnforcers.md b/documents/CaveatEnforcers.md index 43c3b51c..f7a57fb9 100644 --- a/documents/CaveatEnforcers.md +++ b/documents/CaveatEnforcers.md @@ -174,6 +174,139 @@ Note that in this scenario we have the same end recipient (treasury) and the sam If you are delegating to an EOA in a delegation chain, the EOA cannot execute directly since it cannot redeem inner delegations. The EOA can become a deleGator by using EIP7702 or it can use an adapter contract to execute the delegation. An example for that is available in `./src/helpers/DelegationMetaSwapAdapter.sol`. +### ApprovalRevocationEnforcer + +The `ApprovalRevocationEnforcer` lets a delegator grant a delegate the narrow authority to **clear an existing token approval** on the delegator's behalf, without granting any other power over the delegator's assets. It covers six revocation primitives — three standard token-contract primitives and three against the canonical Permit2 deployment: + +- ERC-20 `approve(spender, 0)` +- ERC-721 per-token `approve(address(0), tokenId)` +- ERC-721 / ERC-1155 `setApprovalForAll(operator, false)` (both standards share the selector) +- Permit2 `approve(token, spender, 0, 0)` — single-pair on-chain allowance revocation +- Permit2 `lockdown((address,address)[])` — batched on-chain allowance revocation +- Permit2 `invalidateNonces(token, spender, newNonce)` — invalidate signed-but-unredeemed `permit` payloads + +The Permit2 branches are restricted to the canonical deployment at `0x000000000022D473030F116dDEE9F6B43aC78BA3` (deterministic across mainnet, Base, Arbitrum, Optimism, etc.). On chains where canonical Permit2 is not deployed, do not enable the Permit2 bits — see [Trust Assumptions](#trust-assumptions) below. + +#### Terms + +The enforcer reads a **1-byte bitmask** from `terms` to control which revocation primitives the delegate may use: + +| Bit | Hex mask | Allowed primitive | +|-----|----------|-------------------| +| 0 | `0x01` | ERC-20 `approve(spender, 0)` | +| 1 | `0x02` | ERC-721 `approve(address(0), tokenId)` | +| 2 | `0x04` | `setApprovalForAll(operator, false)` (ERC-721 & ERC-1155) | +| 3 | `0x08` | Permit2 `approve(token, spender, 0, 0)` | +| 4 | `0x10` | Permit2 `lockdown((address,address)[])` | +| 5 | `0x20` | Permit2 `invalidateNonces(token, spender, newNonce)` | +| 6–7 | — | Reserved; MUST be zero | + +- Terms MUST be exactly 1 byte. +- A zero mask (`0x00`) is rejected — at least one primitive must be permitted. +- Any reserved bit (6–7) set is rejected. +- `0x3F` enables all six primitives. + +**Common examples:** + +``` +terms = 0x01 → ERC-20 revocations only +terms = 0x04 → operator (setApprovalForAll) revocations only +terms = 0x08 → single-pair Permit2 revocations only +terms = 0x10 → batched Permit2 revocations only +terms = 0x20 → Permit2 nonce invalidation only +terms = 0x18 → both Permit2 on-chain revocation primitives +terms = 0x38 → all three Permit2 primitives (full Permit2 sever: on-chain allowance + pending signed permits) +terms = 0x3F → all six primitives allowed +``` + +#### Permit2 Revocation Surface + +The three Permit2 primitives target different parts of Permit2's state, and **none of them subsumes the others**: + +| Primitive | Zeros `amount`? | Resets `expiration`? | Bumps `nonce`? | Invalidates pending signed permits? | +|-----------------------|-----------------|---------------------------------|----------------|-------------------------------------| +| `approve(_,_,0,0)` | yes | yes (set to `block.timestamp`) | no | no | +| `lockdown(pairs)` | yes | no | no | no | +| `invalidateNonces(…)` | no | no | yes | yes | + +To **fully sever** a delegator's Permit2 exposure to a `(token, spender)` pair, both an on-chain allowance revocation (bit 3 or 4) **and** a nonce invalidation (bit 5) are typically required. Enabling only on-chain revocation leaves any signed-but-unredeemed `permit` payloads live; enabling only nonce invalidation leaves the existing on-chain allowance intact. Bit-mask `0x38` enables all three. + +> **Note (DoS surface on bit 5).** A delegate granted `invalidateNonces` (bit `0x20`) can advance the stored nonce for any `(token, spender)` pair the caveat does not pin (Permit2 caps the per-call delta at `type(uint16).max`, but a determined delegate can repeat until `nonce == type(uint48).max`, after which the root delegator can no longer sign new permits for that pair). This is never an authority escalation — it can only invalidate, never create — but it is a denial-of-service vector for the delegator's future signed-permit flow. When granting bit 5, pin the `(token, spender)` pair via `AllowedCalldataEnforcer` / `ExactCalldataEnforcer` and/or rate-limit the delegation with `LimitedCallsEnforcer`. + +#### How It Works + +The enforcer runs only in single call type and default execution mode and makes no assumption about the target contract (other than the Permit2 branches, which require the canonical Permit2 address). In `beforeHook` it: + +1. Decodes and validates the 1-byte terms bitmask (rejects empty, zero, or reserved-bit-set terms). +2. Requires the execution to transfer zero native value and to carry at least 4 bytes of calldata. +3. Dispatches by selector and applies the permitted-primitive check (per the bitmask), then branches: + - **Permit2 `approve(address,address,uint160,uint48)`** — requires `target == _PERMIT2`, calldata length `== 132`, `amount == 0`, and `expiration == 0`. No on-chain liveness check is performed. + - **Permit2 `lockdown((address,address)[])`** — requires `target == _PERMIT2`. The calldata is otherwise unconstrained: every entry of the array structurally forces `amount = 0` for the corresponding `(token, spender)` pair (`expiration` and `nonce` are left untouched), so no parameter the delegate could supply can grant new authority. A malformed array reverts inside Permit2 itself. + - **Permit2 `invalidateNonces(address,address,uint48)`** — requires `target == _PERMIT2`. The calldata is otherwise unconstrained: Permit2 enforces strict nonce monotonicity (and a per-call delta capped at `type(uint16).max`), so the call can only invalidate signed-but-unredeemed `permit` payloads, never create or extend an allowance. + - **`setApprovalForAll(address operator, bool approved)`** (calldata length 68) — requires `approved == false` and `isApprovedForAll(delegator, operator) == true` on the target. + - **`approve(address, uint256)`** (calldata length 68, shared by ERC-20 and ERC-721) — disambiguated by the first parameter: + - First parameter is `address(0)` → treated as an ERC-721 per-token revocation; requires `getApproved(tokenId)` on the target to return a non-zero address. + - First parameter is non-zero → treated as an ERC-20 revocation; requires the second parameter (amount) to be zero and `allowance(delegator, spender) > 0` on the target. +4. Reverts on any other selector. + +All six accepted calldatas structurally reduce permissions (amount `0`, spender `address(0)`, `approved` `false`, per-pair Permit2 amount zeroing, or strictly monotonic Permit2 nonce bump). A delegate using this enforcer can therefore **never be granted new authority** over the delegator's assets — only existing approvals can be cleared and pending Permit2 signatures invalidated. + +#### Liveness vs. Race-Freedom + +The ERC-20, ERC-721, and `setApprovalForAll` branches each include a "pre-existing approval" check on the target token contract. This is a liveness / sanity guard ensuring the call is not a no-op at the time the hook runs. It is **not** a race-free invariant: the delegator could independently clear the approval between the hook and the execution. In that case the execution is still safe — it simply becomes a no-op on the token contract. + +The three Permit2 branches intentionally **omit** this on-chain liveness pre-check. Permit2 silently overwrites any existing allowance, so a call against a `(token, spender)` pair with no live allowance is a harmless no-op (or, for `invalidateNonces` against a triple whose stored nonce already meets the new value, reverts inside Permit2 itself). The structural constraints (canonical Permit2 target, fixed selector, and — for `approve` — zero amount and zero expiration) already guarantee the call can only reduce permissions. + +#### Trust Assumptions + +The Permit2 branches assume the canonical Uniswap-deployed Permit2 contract is at `_PERMIT2 = 0x000000000022D473030F116dDEE9F6B43aC78BA3` on the target chain. On chains where Uniswap has deployed Permit2 this is a safe deterministic address. On chains where canonical Permit2 is **not** deployed: + +- if the address is empty, the executor's call returns successfully with no effect (harmless no-op); +- if a *different* contract happens to live at that address, the selector dispatches into whatever that contract does. The `approve(0, 0)` branch is partially self-protected by its structural calldata checks (any contract under that selector would have to interpret the layout identically to grant authority), but `lockdown` and `invalidateNonces` have no such structural moat. + +Delegators on chains without canonical Permit2 should NOT enable bits 3, 4, or 5. + +#### Incompatibility: ERC-20 Tokens That Revert on Zero-Value `approve` + +A small number of non-standard ERC-20 tokens — most notably **BNB on Ethereum mainnet**, and a handful of older tokens — revert when `approve(spender, 0)` is called. Because the ERC-20 branch of this enforcer strictly requires `amount == 0` and provides no alternative revocation primitive (e.g. `decreaseAllowance`), **allowances previously granted on such tokens cannot be revoked through this enforcer**: the executed `approve(spender, 0)` reverts inside the token contract. + +Implications for delegators: + +- Do not rely on a delegation carrying `ApprovalRevocationEnforcer` (bit `0x01`) to clear an outstanding allowance for one of these tokens. +- Be aware before signing a **batch** that includes this enforcer for such a token — the entire batch will revert at the token call. +- Revoke these allowances directly from the owning account (or via a different revocation path) instead. + +The ERC-721, `setApprovalForAll`, and Permit2 branches are unaffected. + +#### Use Cases + +- **Revocation bots / keepers**: Delegate to a third party that can proactively clean up stale or compromised approvals. +- **Post-incident remediation**: Issue a short-lived delegation to revoke a specific approval after a spender contract is found to be malicious. For Permit2, combine bit 3/4 (on-chain) with bit 5 (signature invalidation) to fully sever the spender. +- **User-facing "revoke all" flows**: Let a UI batch revocations on the user's behalf without asking for a new signature per clear. `lockdown` is particularly useful here for clearing many Permit2 allowances in a single transaction; pair it with `invalidateNonces` if the user also wants to kill any outstanding signed permits. + +#### Composition + +The enforcer is intentionally not scoped to any particular spender, operator, or `(token, spender)` pair. To restrict it further, compose it with existing enforcers: + +- `AllowedTargetsEnforcer` — restrict revocation to specific token contracts. Note that for the Permit2 branches the target is already pinned to the canonical Permit2 address by the enforcer itself. +- `AllowedCalldataEnforcer` / `ExactCalldataEnforcer` — pin the exact spender, operator, tokenId, or `(token, spender, newNonce)` triple. For the static branches (`approve`, `setApprovalForAll`, Permit2 `approve`, Permit2 `invalidateNonces`) these compare cleanly against fixed offsets. For Permit2 `lockdown` the calldata is dynamic (offset + array length + entries), so `ExactCalldataEnforcer` is usually the cleaner option for pinning a specific list of pairs. + +#### Redelegation Caveat (Link-Local Semantics) + +The `_delegator` argument passed to `beforeHook` is the delegator of the specific delegation that carries the caveat, **not** the root of a redelegation chain. The `DelegationManager` always executes the downstream call against the root delegator's account. On a root-level delegation (chain length 1) the two are the same and the pre-check queries the account whose storage will actually be mutated — this is the intended usage. + +On an intermediate (redelegation) link the two differ. The implications are different per primitive group: + +- **ERC-20 / ERC-721 / `setApprovalForAll` branches** — the pre-check queries the intermediate delegator's approval state while the execution mutates the root delegator's storage. Concretely: + - If the intermediate delegator has no matching approval, the hook reverts even when the root does (the chain cannot be used, even though the revocation would have been valid for the root). + - If the intermediate delegator happens to have some approval, the hook passes and the execution clears the root's approval regardless of whether the root actually had one to clear. + +- **Permit2 branches** — no per-delegator pre-check is performed. On an intermediate link the link-local sanity guard is simply absent: the hook always passes (subject only to the per-flag and target checks), and the executed call zeros / bumps the root delegator's Permit2 state for whatever `(token, spender)` pair the delegate supplies. Composition with `AllowedCalldataEnforcer` / `ExactCalldataEnforcer` to pin the pair is therefore **load-bearing** — not belt-and-suspenders — for any redelegated Permit2 caveat. + +Neither case is an authority escalation (the structural constraints above still hold — the call can only reduce permissions), but the sanity guard is misaligned with the executed effect for the standard branches and absent entirely for the Permit2 branches. + +If a redelegator needs a root-scoped guarantee (e.g. "Carol may only revoke one of Alice's specific approvals"), they should rely on structural caveats that compose cleanly across links, such as `AllowedTargetsEnforcer`, `AllowedCalldataEnforcer`, or `ExactCalldataEnforcer`. Placing `ApprovalRevocationEnforcer` on an intermediate link in the hope of validating the root's approval state does not achieve that. + ## LogicalOrWrapperEnforcer Context Switching The `LogicalOrWrapperEnforcer` enables logical OR functionality between groups of enforcers, allowing flexibility in delegation constraints. This enforcer is designed for a narrow set of use cases, and careful attention must be given when constructing caveats. The enforcer introduces an important architectural consideration: **context switching**. diff --git a/script/DeployCaveatEnforcers.s.sol b/script/DeployCaveatEnforcers.s.sol index cff28621..629da134 100644 --- a/script/DeployCaveatEnforcers.s.sol +++ b/script/DeployCaveatEnforcers.s.sol @@ -41,8 +41,10 @@ import { ValueLteEnforcer } from "../src/enforcers/ValueLteEnforcer.sol"; import { ERC20MultiOperationIncreaseBalanceEnforcer } from "../src/enforcers/ERC20MultiOperationIncreaseBalanceEnforcer.sol"; import { ERC721MultiOperationIncreaseBalanceEnforcer } from "../src/enforcers/ERC721MultiOperationIncreaseBalanceEnforcer.sol"; import { ERC1155MultiOperationIncreaseBalanceEnforcer } from "../src/enforcers/ERC1155MultiOperationIncreaseBalanceEnforcer.sol"; -import { NativeTokenMultiOperationIncreaseBalanceEnforcer } from - "../src/enforcers/NativeTokenMultiOperationIncreaseBalanceEnforcer.sol"; +import { + NativeTokenMultiOperationIncreaseBalanceEnforcer +} from "../src/enforcers/NativeTokenMultiOperationIncreaseBalanceEnforcer.sol"; +import { ApprovalRevocationEnforcer } from "../src/enforcers/ApprovalRevocationEnforcer.sol"; /** * @title DeployCaveatEnforcers @@ -183,6 +185,9 @@ contract DeployCaveatEnforcers is Script { deployedAddress = address(new NativeTokenMultiOperationIncreaseBalanceEnforcer{ salt: salt }()); console2.log("NativeTokenMultiOperationIncreaseBalanceEnforcer: %s", deployedAddress); + deployedAddress = address(new ApprovalRevocationEnforcer{ salt: salt }()); + console2.log("ApprovalRevocationEnforcer: %s", deployedAddress); + vm.stopBroadcast(); } } diff --git a/script/verification/verify-enforcer-contracts.sh b/script/verification/verify-enforcer-contracts.sh index b9cbc162..ef77e9a1 100755 --- a/script/verification/verify-enforcer-contracts.sh +++ b/script/verification/verify-enforcer-contracts.sh @@ -57,6 +57,7 @@ ENFORCERS=( "ERC721MultiOperationIncreaseBalanceEnforcer" "ERC1155MultiOperationIncreaseBalanceEnforcer" "NativeTokenMultiOperationIncreaseBalanceEnforcer" + "ApprovalRevocationEnforcer" ) ADDRESSES=( @@ -94,6 +95,7 @@ ADDRESSES=( "0x44877cDAFC0d529ab144bb6B0e202eE377C90229" "0x9eB86bbdaA71D4D8d5Fb1B8A9457F04D3344797b" "0xaD551E9b971C1b0c02c577bFfCFAA20b81777276" + "0x0000000000000000000000000000000000000000" ) ############################################################################### diff --git a/src/enforcers/ApprovalRevocationEnforcer.sol b/src/enforcers/ApprovalRevocationEnforcer.sol new file mode 100644 index 00000000..2555479c --- /dev/null +++ b/src/enforcers/ApprovalRevocationEnforcer.sol @@ -0,0 +1,381 @@ +// SPDX-License-Identifier: MIT AND Apache-2.0 +pragma solidity 0.8.23; + +import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import { IERC721 } from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; +import { ExecutionLib } from "@erc7579/lib/ExecutionLib.sol"; + +import { CaveatEnforcer } from "./CaveatEnforcer.sol"; +import { ModeCode } from "../utils/Types.sol"; + +/** + * @title ApprovalRevocationEnforcer + * @notice Allows a delegate to clear existing token approvals. The delegator controls which revocation + * primitives the delegate may perform via a 1-byte bitmask in `terms`: + * + * Bit 0 (`0x01`) — ERC-20 `approve(spender, 0)` (spender non-zero, amount zero) + * Bit 1 (`0x02`) — ERC-721 per-token `approve(address(0), tokenId)` + * Bit 2 (`0x04`) — ERC-721 / ERC-1155 `setApprovalForAll(operator, false)` + * Bit 3 (`0x08`) — Permit2 `approve(token, spender, 0, 0)` against the canonical Permit2 deployment + * Bit 4 (`0x10`) — Permit2 `lockdown((address,address)[])` against the canonical Permit2 deployment + * Bit 5 (`0x20`) — Permit2 `invalidateNonces(token, spender, newNonce)` against the canonical Permit2 deployment + * Bits 6-7 — Reserved; MUST be zero. + * + * Examples: + * `0x01` — delegate may only clear ERC-20 allowances. + * `0x04` — delegate may only revoke operator approvals. + * `0x08` — delegate may only revoke a single Permit2 allowance per call. + * `0x10` — delegate may only batch-revoke Permit2 allowances via `lockdown`. + * `0x20` — delegate may only invalidate Permit2 nonces (kill pending signed permits). + * `0x38` — delegate may use all three Permit2 primitives (`approve` + `lockdown` + `invalidateNonces`), + * which together fully sever a Permit2 (token, spender) exposure (on-chain allowance + pending + * signed permits). + * `0x3F` — delegate may use all six revocation primitives. + * + * Terms MUST be exactly 1 byte, MUST not be zero, and MUST NOT set any reserved bit. + * + * @dev ERC-721 and ERC-1155 intentionally share the `setApprovalForAll(address,bool)` selector; this enforcer + * handles both via the `IERC721` interface (the selector and ABI are identical, so a typed `IERC1155` import is + * unnecessary for the external call). ERC-20 and ERC-721 likewise share the `approve(address,uint256)` selector, + * and are disambiguated by inspecting the first parameter (see branching rules below). + * + * @dev The execution must transfer zero native value and carry one of the supported approval calldatas. Branching + * is determined as follows: + * - selector `approve(address token, address spender, uint160 amount, uint48 expiration)` (Permit2): + * - `target` MUST equal the canonical Permit2 deployment (`_PERMIT2`), and + * - calldata length MUST be 132 bytes (4-byte selector + four 32-byte words), and + * - the third parameter (amount) MUST be zero, and + * - the fourth parameter (expiration) MUST be zero. + * - selector `lockdown((address,address)[])` (Permit2): + * - `target` MUST equal the canonical Permit2 deployment (`_PERMIT2`). + * - The calldata is otherwise unconstrained: every entry of the array structurally forces the corresponding + * `(token, spender)` Permit2 allowance `amount` to zero (`expiration` and `nonce` are left untouched). There + * is no parameter the delegate could supply to grant new authority, so no further calldata validation is + * performed. + * - selector `invalidateNonces(address,address,uint48)` (Permit2): + * - `target` MUST equal the canonical Permit2 deployment (`_PERMIT2`). + * - The calldata is otherwise unconstrained: Permit2's `invalidateNonces` strictly monotonically increases the + * stored nonce for the `(caller, token, spender)` triple (it reverts if `newNonce <= oldNonce` and caps the + * per-call delta at `type(uint16).max`). It cannot create or extend an allowance, so no further calldata + * validation is performed. + * - calldata length 68 bytes (4-byte selector + two 32-byte words), shared by `approve(address,uint256)` and + * `setApprovalForAll(address,bool)`: + * - selector `setApprovalForAll(address operator, bool approved)`: + * - `approved` MUST be false, and + * - `isApprovedForAll(delegator, operator)` MUST currently be true on the target. + * - selector `approve(address, uint256)` (shared by ERC-20 and ERC-721): + * - if the first parameter is `address(0)` the call is treated as an ERC-721 per-token revocation: + * - `getApproved(tokenId)` on the target MUST currently return a non-zero address. + * - otherwise the call is treated as an ERC-20 revocation: + * - the second parameter (amount) MUST be zero, and + * - `allowance(delegator, spender)` on the target MUST currently return non-zero. + * + * @dev All six accepted calldatas structurally result in a net reduction of permissions on the target (amount + * `0`, spender `address(0)`, `approved` `false`, per-pair Permit2 amount zeroing, or strictly monotonic Permit2 + * nonce bump). A delegate using this enforcer can therefore never be granted new authority over the delegator's + * assets — only existing approvals can be cleared and pending Permit2 signatures invalidated. + * + * @dev Unlike the ERC-20 / ERC-721 / `setApprovalForAll` primitives, the three Permit2 branches perform no + * on-chain liveness pre-check. The structural constraints (canonical Permit2 target, fixed selector, and — for + * `approve` — zero amount and zero expiration) already guarantee the call can only reduce permissions; if no + * Permit2 state exists for the targeted `(token, spender)` pair(s) the execution is either a harmless no-op or + * (for `invalidateNonces`) reverts inside Permit2. Restrict which pairs the delegate may target by composing + * this enforcer with `AllowedCalldataEnforcer` or `ExactCalldataEnforcer`. Note that for `lockdown` such pinning + * also has to fix the array length and ABI head words, since the calldata is dynamic; `ExactCalldataEnforcer` + * is usually the cleaner option there. + * + * @dev Permit2 revocation surface — what each primitive does and does not cover: + * - `approve(token, spender, 0, 0)` zeros `amount` and sets `expiration` to `block.timestamp` for the caller's + * `(token, spender)` allowance. Pending signed permits are NOT invalidated (their `nonce` is unaffected). + * - `lockdown` zeros `amount` only. `expiration` and `nonce` are unchanged. Pending signed permits are NOT + * invalidated. + * - `invalidateNonces` strictly monotonically increases the stored `nonce`, rendering all signed-but-unredeemed + * `permit` payloads with a now-stale nonce uncollectable. It does NOT zero on-chain `amount` or `expiration`. + * To fully sever Permit2 exposure to a spender, both an on-chain allowance revocation (bit 3 or 4) AND a + * nonce invalidation (bit 5) are typically required. Enabling only one leaves the other vector live. + * + * @dev DoS surface on bit 5 (`invalidateNonces`). A delegate granted bit 5 can advance the stored nonce for any + * `(token, spender)` pair the caveat does not pin (Permit2 caps the per-call delta at `type(uint16).max`, but a + * determined delegate can repeat until `nonce == type(uint48).max`, after which the root delegator can no longer + * sign new permits for that pair). This is never an authority escalation — it can only invalidate, never create — + * but it is a denial-of-service vector for the delegator's future signed-permit flow. When granting bit 5, pin the + * `(token, spender)` pair via `AllowedCalldataEnforcer` / `ExactCalldataEnforcer` and/or rate-limit the delegation + * with `LimitedCallsEnforcer`. + * + * @dev Trust assumption — canonical Permit2 deployment. + * + * The Permit2 branches assume the canonical Uniswap-deployed Permit2 contract is at `_PERMIT2` on the target + * chain. On chains where Uniswap has deployed Permit2 (mainnet, Base, Arbitrum, Optimism, Polygon, BNB, Avalanche, + * etc.) this is a safe deterministic address. On chains where canonical Permit2 is NOT deployed: + * - if the address is empty, the executor's call returns successfully with no effect (harmless no-op); + * - if a *different* contract happens to live at that address, the selector dispatches into whatever that + * contract does. The `approve(0, 0)` branch is partially self-protected by its structural calldata checks + * (any contract under that selector would have to interpret the layout identically to grant authority), but + * `lockdown` and `invalidateNonces` have no such structural moat. + * Delegators on chains without canonical Permit2 should NOT enable bits 3, 4, or 5. + * + * @dev REDELEGATION WARNING — link-local pre-check vs. root-level execution. + * + * The `_delegator` argument passed to `beforeHook` is the delegator of the specific delegation that carries the + * caveat, not the root of a redelegation chain. The DelegationManager always executes the downstream call against + * the *root* delegator's account (the account at the end of the leaf-to-root chain). On a root-level delegation + * (chain length 1) the two are the same and the pre-check queries the account whose storage will actually be + * mutated — this is the intended usage. + * + * On an intermediate (redelegation) link the two differ. The implications are different per primitive group: + * + * (a) ERC-20 / ERC-721 / `setApprovalForAll` branches — pre-check is link-local. + * + * The pre-check queries the *intermediate* delegator's approval state, while the execution mutates the *root* + * delegator's storage. A redelegator adding this caveat to constrain their delegate is very likely expecting the + * pre-check to run against the root. That expectation is wrong — the check is link-local. + * + * Concrete example. Alice -> Bob -> Carol. Alice's link has no caveat (Bob has full authority over Alice). + * Bob places this enforcer on his delegation to Carol, intending "Carol can only revoke an existing approval on + * Alice's account". When Carol redeems, the hook fires with `_delegator = Bob`, not Alice, so: + * - if Bob has no allowance to the same spender on the target, the hook reverts even when Alice does have + * one (Carol cannot use the chain, even though the revocation would have been valid for Alice); + * - if Bob happens to have some allowance, the hook passes and the execution clears Alice's allowance — + * independently of whether Alice actually had an allowance to clear. + * + * (b) Permit2 branches — no pre-check at all. + * + * The Permit2 branches do not consult `_delegator` (no on-chain liveness check is performed). On an intermediate + * link this means the link-local sanity guard that exists for the ERC-20/721/operator branches is simply absent: + * the hook always passes (subject only to the per-flag and target checks), and the executed call zeros / bumps + * the *root* delegator's Permit2 state for whatever `(token, spender)` pair the delegate supplies. + * + * Neither (a) nor (b) is an authority escalation (the structural constraints above still apply — the call can + * only reduce permissions). But the sanity guard is misaligned with the executed effect, and for the Permit2 + * branches it is absent entirely. Composition with `AllowedCalldataEnforcer` / `ExactCalldataEnforcer` to pin the + * `(token, spender)` pair is therefore load-bearing for any redelegated Permit2 caveat. + * + * If a redelegator needs a root-scoped guarantee (e.g. "Carol may only revoke one of Alice's specific + * approvals") they should rely on structural caveats that compose cleanly across links, such as + * `AllowedTargetsEnforcer` (restrict which token contract), `AllowedCalldataEnforcer` (pin the exact spender + * or tokenId), or `ExactCalldataEnforcer`. Placing `ApprovalRevocationEnforcer` on an intermediate link in the + * hope of validating the root's approval state does not achieve that. + * + * @dev The "pre-existing approval" check is a liveness/sanity guard ensuring the call is not a no-op at the time + * the hook runs. It is not a race-free invariant: the delegator could independently clear the approval between + * the hook and the execution. In that case the execution is still safe — it simply becomes a no-op. + * + * @dev Delegators who want to restrict revocation to specific tokens should compose this enforcer with + * `AllowedTargetsEnforcer`. + * + * @dev INCOMPATIBILITY — ERC-20 tokens that revert on zero-value `approve`. A small number of non-standard + * ERC-20 tokens (notably BNB on Ethereum mainnet, and a handful of older tokens) revert when `approve(spender, 0)` + * is called. Because the ERC-20 branch of this enforcer strictly requires `amount == 0` and provides no alternative + * revocation primitive (e.g. `decreaseAllowance`), allowances previously granted on such tokens CANNOT be revoked + * through this enforcer — the executed `approve(spender, 0)` will revert inside the token contract. Delegators + * holding these tokens should revoke their allowances directly from the owning account (or via a different + * revocation path), and should be aware before signing a delegation or batch that includes this enforcer for + * such a token. The Permit2, ERC-721, and `setApprovalForAll` branches are unaffected. + * + * @dev This enforcer operates only in single call type and default execution mode. + */ +contract ApprovalRevocationEnforcer is CaveatEnforcer { + using ExecutionLib for bytes; + + ////////////////////////////// Constants ////////////////////////////// + + /** + * @dev Permission flags packed into the single-byte terms bitmask. + */ + uint8 internal constant _PERMISSION_ERC20_APPROVE = 0x01; + uint8 internal constant _PERMISSION_ERC721_APPROVE = 0x02; + uint8 internal constant _PERMISSION_SET_APPROVAL_FOR_ALL = 0x04; + uint8 internal constant _PERMISSION_PERMIT2_APPROVE = 0x08; + uint8 internal constant _PERMISSION_PERMIT2_LOCKDOWN = 0x10; + uint8 internal constant _PERMISSION_PERMIT2_INVALIDATE_NONCES = 0x20; + uint8 internal constant _PERMISSION_MASK = _PERMISSION_ERC20_APPROVE | _PERMISSION_ERC721_APPROVE + | _PERMISSION_SET_APPROVAL_FOR_ALL | _PERMISSION_PERMIT2_APPROVE | _PERMISSION_PERMIT2_LOCKDOWN + | _PERMISSION_PERMIT2_INVALIDATE_NONCES; + + /** + * @dev Canonical Permit2 deployment address (deterministic across EVM chains where Uniswap has deployed it, + * e.g. mainnet, Base, Arbitrum, Optimism, etc.). See the contract-level "Trust assumption" NatSpec for the + * implications on chains where canonical Permit2 is not deployed. + */ + address internal constant _PERMIT2 = 0x000000000022D473030F116dDEE9F6B43aC78BA3; + + /** + * @dev `bytes4(keccak256("approve(address,address,uint160,uint48)"))` — Permit2's `approve` selector. + */ + bytes4 internal constant _PERMIT2_APPROVE_SELECTOR = 0x87517c45; + + /** + * @dev `bytes4(keccak256("lockdown((address,address)[])"))` — Permit2's batch revocation selector. Every entry + * of the array unconditionally zeros `amount` for the corresponding `(token, spender)` pair on the caller; + * `expiration` and `nonce` are left untouched. No parameter can be used to grant authority. + */ + bytes4 internal constant _PERMIT2_LOCKDOWN_SELECTOR = 0xcc53287f; + + /** + * @dev `bytes4(keccak256("invalidateNonces(address,address,uint48)"))` — Permit2's nonce-invalidation + * selector. The new nonce is required by Permit2 to be strictly greater than the current nonce (with a + * per-call delta capped at `type(uint16).max`); it can therefore only invalidate signed-but-unredeemed + * `permit` payloads, never create or extend an allowance. + */ + bytes4 internal constant _PERMIT2_INVALIDATE_NONCES_SELECTOR = 0x65d9723c; + + ////////////////////////////// Public Methods ////////////////////////////// + + /** + * @notice Requires the execution to revoke an existing token approval owned by `_delegator`, and that the + * revocation primitive used is permitted by `_terms`. + * @param _terms 1-byte bitmask selecting which revocation primitives are allowed. See the contract NatSpec. + * @param _mode Must be single call type and default execution mode. + * @param _executionCallData Single execution targeting the token contract. + * @param _delegator The delegator of the delegation carrying this caveat (link-local, not the chain root). + * See the contract-level NatSpec for the implications in redelegation chains. + */ + function beforeHook( + bytes calldata _terms, + bytes calldata, + ModeCode _mode, + bytes calldata _executionCallData, + bytes32, + address _delegator, + address + ) + public + view + override + onlySingleCallTypeMode(_mode) + onlyDefaultExecutionMode(_mode) + { + // Validate terms and capture the raw flags byte (1 stack slot vs. 3 bools). + uint8 flags_ = _parseFlags(_terms); + + (address target_, uint256 value_, bytes calldata callData_) = _executionCallData.decodeSingle(); + + require(value_ == 0, "ApprovalRevocationEnforcer:invalid-value"); + require(callData_.length >= 4, "ApprovalRevocationEnforcer:invalid-execution-length"); + + bytes4 selector_ = bytes4(callData_[0:4]); + + // Permit2 `approve(address,address,uint160,uint48)`: 4 + 4*32 = 132 bytes. Dispatched first because it has + // its own length and selector and never overlaps with the other primitives. + if (selector_ == _PERMIT2_APPROVE_SELECTOR) { + require(flags_ & _PERMISSION_PERMIT2_APPROVE != 0, "ApprovalRevocationEnforcer:permit2-approve-not-allowed"); + _validatePermit2Approve(target_, callData_); + return; + } + + // Permit2 `lockdown((address,address)[])`: dynamic calldata. Only the canonical Permit2 target is + // enforced — every entry of the array structurally zeros a Permit2 allowance amount, so no + // calldata-shape validation is needed (a malformed payload simply reverts inside Permit2 itself). + if (selector_ == _PERMIT2_LOCKDOWN_SELECTOR) { + require(flags_ & _PERMISSION_PERMIT2_LOCKDOWN != 0, "ApprovalRevocationEnforcer:permit2-lockdown-not-allowed"); + require(target_ == _PERMIT2, "ApprovalRevocationEnforcer:invalid-permit2-target"); + return; + } + + // Permit2 `invalidateNonces(address,address,uint48)`: only the canonical Permit2 target is enforced. + // Permit2 itself enforces strict nonce monotonicity (and a per-call uint16-bounded delta), so the call + // can only invalidate signed-but-unredeemed permits, never create authority. + if (selector_ == _PERMIT2_INVALIDATE_NONCES_SELECTOR) { + require( + flags_ & _PERMISSION_PERMIT2_INVALIDATE_NONCES != 0, + "ApprovalRevocationEnforcer:permit2-invalidate-nonces-not-allowed" + ); + require(target_ == _PERMIT2, "ApprovalRevocationEnforcer:invalid-permit2-target"); + return; + } + + // 68 = 4-byte selector + two 32-byte words. Shared by `approve(address,uint256)` and + // `setApprovalForAll(address,bool)`. + require(callData_.length == 68, "ApprovalRevocationEnforcer:invalid-execution-length"); + + if (selector_ == IERC721.setApprovalForAll.selector) { + require(flags_ & _PERMISSION_SET_APPROVAL_FOR_ALL != 0, "ApprovalRevocationEnforcer:setApprovalForAll-not-allowed"); + _validateOperatorRevocation(target_, callData_, _delegator); + return; + } + if (selector_ == IERC20.approve.selector) { + // ERC-20 and ERC-721 share `approve(address,uint256)`. Disambiguate by the first parameter: ERC-721 + // revokes via `approve(address(0), tokenId)`, while ERC-20 revokes via `approve(spender, 0)` with a + // non-zero spender. + address firstParam_ = address(uint160(uint256(bytes32(callData_[4:36])))); + if (firstParam_ == address(0)) { + require(flags_ & _PERMISSION_ERC721_APPROVE != 0, "ApprovalRevocationEnforcer:erc721-approve-not-allowed"); + _validateErc721Revocation(target_, callData_); + } else { + require(flags_ & _PERMISSION_ERC20_APPROVE != 0, "ApprovalRevocationEnforcer:erc20-approve-not-allowed"); + _validateErc20Revocation(target_, callData_, _delegator, firstParam_); + } + return; + } + revert("ApprovalRevocationEnforcer:invalid-method"); + } + + ////////////////////////////// Internal Methods ////////////////////////////// + + /** + * @dev Validates and returns the raw permission flags byte. Reverts on invalid terms. + */ + function _parseFlags(bytes calldata _terms) private pure returns (uint8 flags_) { + require(_terms.length == 1, "ApprovalRevocationEnforcer:invalid-terms-length"); + flags_ = uint8(_terms[0]); + require(flags_ != 0, "ApprovalRevocationEnforcer:no-methods-allowed"); + require(flags_ & ~_PERMISSION_MASK == 0, "ApprovalRevocationEnforcer:invalid-terms"); + } + + /** + * @dev Validates an ERC-20 `approve(spender, 0)` revocation. Requires `allowance(delegator, spender) > 0` on + * the target. + */ + function _validateErc20Revocation( + address _target, + bytes calldata _callData, + address _delegator, + address _spender + ) + private + view + { + require(uint256(bytes32(_callData[36:68])) == 0, "ApprovalRevocationEnforcer:non-zero-amount"); + + require(IERC20(_target).allowance(_delegator, _spender) != 0, "ApprovalRevocationEnforcer:no-approval-to-revoke"); + } + + /** + * @dev Validates an ERC-721 `approve(address(0), tokenId)` revocation. Requires `getApproved(tokenId)` on the + * target to be non-zero (i.e. an approval is currently set). + */ + function _validateErc721Revocation(address _target, bytes calldata _callData) private view { + uint256 tokenId_ = uint256(bytes32(_callData[36:68])); + + require(IERC721(_target).getApproved(tokenId_) != address(0), "ApprovalRevocationEnforcer:no-approval-to-revoke"); + } + + /** + * @dev Validates a `setApprovalForAll(operator, false)` revocation (ERC-721 and ERC-1155 share this selector). + * Requires `isApprovedForAll(delegator, operator)` on the target to currently be true. + */ + function _validateOperatorRevocation(address _target, bytes calldata _callData, address _delegator) private view { + require(uint256(bytes32(_callData[36:68])) == 0, "ApprovalRevocationEnforcer:not-a-revocation"); + + address operator_ = address(uint160(uint256(bytes32(_callData[4:36])))); + require(IERC721(_target).isApprovedForAll(_delegator, operator_), "ApprovalRevocationEnforcer:no-approval-to-revoke"); + } + + /** + * @dev Validates a Permit2 `approve(token, spender, 0, 0)` revocation. Requires the target to be the canonical + * Permit2 deployment, the calldata to be exactly 132 bytes, and both `amount` (uint160) and `expiration` + * (uint48) parameters to be zero. No on-chain liveness check is performed: Permit2 silently overwrites any + * existing allowance, so calling against a (token, spender) pair with no live allowance is a harmless no-op. + * The first two parameters (token, spender) are intentionally unconstrained here — compose with + * `AllowedCalldataEnforcer` if a particular pair must be enforced. + */ + function _validatePermit2Approve(address _target, bytes calldata _callData) private pure { + require(_target == _PERMIT2, "ApprovalRevocationEnforcer:invalid-permit2-target"); + require(_callData.length == 132, "ApprovalRevocationEnforcer:permit2-invalid-execution-length"); + // amount (uint160) sits in the 3rd word; it is ABI-encoded with 12 bytes of left padding (right-aligned + // in the word), so checking the full 32-byte word for zero is equivalent and cheaper. + require(uint256(bytes32(_callData[68:100])) == 0, "ApprovalRevocationEnforcer:non-zero-amount"); + // expiration (uint48, in the 4th word) MUST be zero. + require(uint256(bytes32(_callData[100:132])) == 0, "ApprovalRevocationEnforcer:non-zero-expiration"); + } +} diff --git a/test/enforcers/ApprovalRevocationEnforcer.t.sol b/test/enforcers/ApprovalRevocationEnforcer.t.sol new file mode 100644 index 00000000..94889378 --- /dev/null +++ b/test/enforcers/ApprovalRevocationEnforcer.t.sol @@ -0,0 +1,899 @@ +// SPDX-License-Identifier: MIT AND Apache-2.0 +pragma solidity 0.8.23; + +import "forge-std/Test.sol"; +import { ExecutionLib } from "@erc7579/lib/ExecutionLib.sol"; +import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import { IERC721 } from "@openzeppelin/contracts/token/ERC721/IERC721.sol"; +import { IERC1155 } from "@openzeppelin/contracts/token/ERC1155/IERC1155.sol"; + +import { Execution, Caveat, Delegation, ModeCode } from "../../src/utils/Types.sol"; +import { CaveatEnforcerBaseTest } from "./CaveatEnforcerBaseTest.t.sol"; +import { BasicERC20 } from "../utils/BasicERC20.t.sol"; +import { BasicCF721 } from "../utils/BasicCF721.t.sol"; +import { BasicERC1155 } from "../utils/BasicERC1155.t.sol"; +import { ICaveatEnforcer } from "../../src/interfaces/ICaveatEnforcer.sol"; +import { ApprovalRevocationEnforcer } from "../../src/enforcers/ApprovalRevocationEnforcer.sol"; +import { EncoderLib } from "../../src/libraries/EncoderLib.sol"; + +/** + * @title ApprovalRevocationEnforcer Test + */ +contract ApprovalRevocationEnforcerTest is CaveatEnforcerBaseTest { + ////////////////////////////// State ////////////////////////////// + + ApprovalRevocationEnforcer public enforcer; + BasicERC20 public erc20; + BasicCF721 public erc721; + BasicERC1155 public erc1155; + + address public delegator; + address public spender; + address public operator; + + uint256 public mintedTokenId; + + /// @dev Permission flag constants mirroring the contract. + uint8 internal constant PERMISSION_ERC20_APPROVE = 0x01; + uint8 internal constant PERMISSION_ERC721_APPROVE = 0x02; + uint8 internal constant PERMISSION_SET_APPROVAL_FOR_ALL = 0x04; + uint8 internal constant PERMISSION_PERMIT2_APPROVE = 0x08; + uint8 internal constant PERMISSION_PERMIT2_LOCKDOWN = 0x10; + uint8 internal constant PERMISSION_PERMIT2_INVALIDATE_NONCES = 0x20; + uint8 internal constant PERMISSION_ALL = 0x3F; + + /// @dev Mirrors the contract constants. + address internal constant PERMIT2 = 0x000000000022D473030F116dDEE9F6B43aC78BA3; + bytes4 internal constant PERMIT2_APPROVE_SELECTOR = 0x87517c45; + bytes4 internal constant PERMIT2_LOCKDOWN_SELECTOR = 0xcc53287f; + bytes4 internal constant PERMIT2_INVALIDATE_NONCES_SELECTOR = 0x65d9723c; + + ////////////////////////////// Set up ////////////////////////////// + + function setUp() public override { + super.setUp(); + enforcer = new ApprovalRevocationEnforcer(); + vm.label(address(enforcer), "ApprovalRevocationEnforcer"); + + delegator = address(users.alice.deleGator); + spender = address(users.bob.deleGator); + operator = address(users.carol.deleGator); + + erc20 = new BasicERC20(delegator, "TestToken", "TT", 100 ether); + erc721 = new BasicCF721(delegator, "TestNFT", "TNFT", ""); + erc1155 = new BasicERC1155(delegator, "Test1155", "T1155", ""); + + vm.label(address(erc20), "BasicERC20"); + vm.label(address(erc721), "BasicCF721"); + vm.label(address(erc1155), "BasicERC1155"); + + // Mint an ERC-721 token to the delegator and approve it. + vm.startPrank(delegator); + erc721.mint(delegator); + mintedTokenId = 0; + erc721.approve(spender, mintedTokenId); + + // ERC-20 allowance. + erc20.approve(spender, 42 ether); + + // setApprovalForAll on both ERC-721 and ERC-1155. + erc721.setApprovalForAll(operator, true); + erc1155.setApprovalForAll(operator, true); + vm.stopPrank(); + } + + ////////////////////////////// Helpers ////////////////////////////// + + function _terms(uint8 _flags) internal pure returns (bytes memory) { + return abi.encodePacked(_flags); + } + + function _approveCallData(address _spender, uint256 _amount) internal pure returns (bytes memory) { + return abi.encodeWithSelector(IERC20.approve.selector, _spender, _amount); + } + + function _setApprovalForAllCallData(address _operator, bool _approved) internal pure returns (bytes memory) { + return abi.encodeWithSelector(IERC721.setApprovalForAll.selector, _operator, _approved); + } + + function _permit2ApproveCallData(address _token, address _spender, uint160 _amount, uint48 _expiration) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector(PERMIT2_APPROVE_SELECTOR, _token, _spender, _amount, _expiration); + } + + /// @dev Mirrors Permit2's `TokenSpenderPair { address token; address spender; }`. Defined locally to avoid a + /// direct Permit2 dependency. + struct TokenSpenderPair { + address token; + address spender; + } + + function _permit2LockdownCallData(TokenSpenderPair[] memory _pairs) internal pure returns (bytes memory) { + return abi.encodeWithSelector(PERMIT2_LOCKDOWN_SELECTOR, _pairs); + } + + function _singlePair(address _token, address _spender) internal pure returns (TokenSpenderPair[] memory pairs_) { + pairs_ = new TokenSpenderPair[](1); + pairs_[0] = TokenSpenderPair({ token: _token, spender: _spender }); + } + + function _permit2InvalidateNoncesCallData(address _token, address _spender, uint48 _newNonce) + internal + pure + returns (bytes memory) + { + return abi.encodeWithSelector(PERMIT2_INVALIDATE_NONCES_SELECTOR, _token, _spender, _newNonce); + } + + function _encodeSingle(address _target, uint256 _value, bytes memory _callData) internal pure returns (bytes memory) { + return ExecutionLib.encodeSingle(_target, _value, _callData); + } + + function _callBeforeHook(bytes memory _termsBytes, bytes memory _executionCallData) internal { + vm.prank(address(delegationManager)); + enforcer.beforeHook(_termsBytes, hex"", singleDefaultMode, _executionCallData, bytes32(0), delegator, address(0)); + } + + function _expectRevertBeforeHook(bytes memory _termsBytes, bytes memory _executionCallData, bytes memory _revertReason) internal { + vm.prank(address(delegationManager)); + vm.expectRevert(_revertReason); + enforcer.beforeHook(_termsBytes, hex"", singleDefaultMode, _executionCallData, bytes32(0), delegator, address(0)); + } + + ////////////////////////////// Terms decoding ////////////////////////////// + + function test_terms_revertOnEmptyTerms() public { + bytes memory executionCallData_ = _encodeSingle(address(erc20), 0, _approveCallData(spender, 0)); + _expectRevertBeforeHook(hex"", executionCallData_, "ApprovalRevocationEnforcer:invalid-terms-length"); + } + + function test_terms_revertOnWrongLength() public { + bytes memory executionCallData_ = _encodeSingle(address(erc20), 0, _approveCallData(spender, 0)); + _expectRevertBeforeHook(abi.encodePacked(uint16(0x0007)), executionCallData_, "ApprovalRevocationEnforcer:invalid-terms-length"); + } + + function test_terms_revertOnZeroMask() public { + bytes memory executionCallData_ = _encodeSingle(address(erc20), 0, _approveCallData(spender, 0)); + _expectRevertBeforeHook(_terms(0x00), executionCallData_, "ApprovalRevocationEnforcer:no-methods-allowed"); + } + + function test_terms_revertOnReservedBitSet_bit6() public { + bytes memory executionCallData_ = _encodeSingle(address(erc20), 0, _approveCallData(spender, 0)); + _expectRevertBeforeHook(_terms(0x40), executionCallData_, "ApprovalRevocationEnforcer:invalid-terms"); + } + + function test_terms_revertOnReservedBitSet_highBit() public { + bytes memory executionCallData_ = _encodeSingle(address(erc20), 0, _approveCallData(spender, 0)); + _expectRevertBeforeHook(_terms(0x80), executionCallData_, "ApprovalRevocationEnforcer:invalid-terms"); + } + + function test_terms_revertOnReservedBitSet_allBits() public { + bytes memory executionCallData_ = _encodeSingle(address(erc20), 0, _approveCallData(spender, 0)); + _expectRevertBeforeHook(_terms(0xFF), executionCallData_, "ApprovalRevocationEnforcer:invalid-terms"); + } + + ////////////////////////////// Per-flag gating ////////////////////////////// + + function test_terms_onlyErc20_allowsErc20() public { + bytes memory executionCallData_ = _encodeSingle(address(erc20), 0, _approveCallData(spender, 0)); + _callBeforeHook(_terms(PERMISSION_ERC20_APPROVE), executionCallData_); + } + + function test_terms_onlyErc20_blocksErc721Approve() public { + bytes memory executionCallData_ = _encodeSingle(address(erc721), 0, _approveCallData(address(0), mintedTokenId)); + _expectRevertBeforeHook(_terms(PERMISSION_ERC20_APPROVE), executionCallData_, "ApprovalRevocationEnforcer:erc721-approve-not-allowed"); + } + + function test_terms_onlyErc20_blocksSetApprovalForAll() public { + bytes memory executionCallData_ = _encodeSingle(address(erc721), 0, _setApprovalForAllCallData(operator, false)); + _expectRevertBeforeHook(_terms(PERMISSION_ERC20_APPROVE), executionCallData_, "ApprovalRevocationEnforcer:setApprovalForAll-not-allowed"); + } + + function test_terms_onlyErc721Approve_allowsErc721() public { + bytes memory executionCallData_ = _encodeSingle(address(erc721), 0, _approveCallData(address(0), mintedTokenId)); + _callBeforeHook(_terms(PERMISSION_ERC721_APPROVE), executionCallData_); + } + + function test_terms_onlyErc721Approve_blocksErc20() public { + bytes memory executionCallData_ = _encodeSingle(address(erc20), 0, _approveCallData(spender, 0)); + _expectRevertBeforeHook(_terms(PERMISSION_ERC721_APPROVE), executionCallData_, "ApprovalRevocationEnforcer:erc20-approve-not-allowed"); + } + + function test_terms_onlyErc721Approve_blocksSetApprovalForAll() public { + bytes memory executionCallData_ = _encodeSingle(address(erc721), 0, _setApprovalForAllCallData(operator, false)); + _expectRevertBeforeHook(_terms(PERMISSION_ERC721_APPROVE), executionCallData_, "ApprovalRevocationEnforcer:setApprovalForAll-not-allowed"); + } + + function test_terms_onlySetApprovalForAll_allowsErc721() public { + bytes memory executionCallData_ = _encodeSingle(address(erc721), 0, _setApprovalForAllCallData(operator, false)); + _callBeforeHook(_terms(PERMISSION_SET_APPROVAL_FOR_ALL), executionCallData_); + } + + function test_terms_onlySetApprovalForAll_allowsErc1155() public { + bytes memory executionCallData_ = _encodeSingle(address(erc1155), 0, _setApprovalForAllCallData(operator, false)); + _callBeforeHook(_terms(PERMISSION_SET_APPROVAL_FOR_ALL), executionCallData_); + } + + function test_terms_onlySetApprovalForAll_blocksErc20Approve() public { + bytes memory executionCallData_ = _encodeSingle(address(erc20), 0, _approveCallData(spender, 0)); + _expectRevertBeforeHook(_terms(PERMISSION_SET_APPROVAL_FOR_ALL), executionCallData_, "ApprovalRevocationEnforcer:erc20-approve-not-allowed"); + } + + function test_terms_onlySetApprovalForAll_blocksErc721Approve() public { + bytes memory executionCallData_ = _encodeSingle(address(erc721), 0, _approveCallData(address(0), mintedTokenId)); + _expectRevertBeforeHook(_terms(PERMISSION_SET_APPROVAL_FOR_ALL), executionCallData_, "ApprovalRevocationEnforcer:erc721-approve-not-allowed"); + } + + function test_terms_pair_erc20AndErc721Approve_blocksSetApprovalForAll() public { + uint8 flags_ = PERMISSION_ERC20_APPROVE | PERMISSION_ERC721_APPROVE; + // Both approve variants allowed. + _callBeforeHook(_terms(flags_), _encodeSingle(address(erc20), 0, _approveCallData(spender, 0))); + _callBeforeHook(_terms(flags_), _encodeSingle(address(erc721), 0, _approveCallData(address(0), mintedTokenId))); + // setApprovalForAll blocked. + _expectRevertBeforeHook(_terms(flags_), _encodeSingle(address(erc721), 0, _setApprovalForAllCallData(operator, false)), "ApprovalRevocationEnforcer:setApprovalForAll-not-allowed"); + } + + function test_terms_pair_erc20AndSetApprovalForAll_blocksErc721Approve() public { + uint8 flags_ = PERMISSION_ERC20_APPROVE | PERMISSION_SET_APPROVAL_FOR_ALL; + _callBeforeHook(_terms(flags_), _encodeSingle(address(erc20), 0, _approveCallData(spender, 0))); + _callBeforeHook(_terms(flags_), _encodeSingle(address(erc721), 0, _setApprovalForAllCallData(operator, false))); + _expectRevertBeforeHook(_terms(flags_), _encodeSingle(address(erc721), 0, _approveCallData(address(0), mintedTokenId)), "ApprovalRevocationEnforcer:erc721-approve-not-allowed"); + } + + ////////////////////////////// Valid cases (ERC-20 approve) ////////////////////////////// + + function test_erc20_revokeSucceedsForExistingAllowance() public { + bytes memory executionCallData_ = _encodeSingle(address(erc20), 0, _approveCallData(spender, 0)); + _callBeforeHook(_terms(PERMISSION_ALL), executionCallData_); + } + + function test_erc20_revokeSucceedsForOneWeiAllowance() public { + address other_ = address(users.dave.deleGator); + vm.prank(delegator); + erc20.approve(other_, 1); + bytes memory executionCallData_ = _encodeSingle(address(erc20), 0, _approveCallData(other_, 0)); + _callBeforeHook(_terms(PERMISSION_ALL), executionCallData_); + } + + ////////////////////////////// Invalid cases (ERC-20 approve) ////////////////////////////// + + function test_erc20_revertOnNonZeroAmount() public { + bytes memory executionCallData_ = _encodeSingle(address(erc20), 0, _approveCallData(spender, 1)); + _expectRevertBeforeHook(_terms(PERMISSION_ALL), executionCallData_, "ApprovalRevocationEnforcer:non-zero-amount"); + } + + function test_erc20_revertWhenNoApproval() public { + address other_ = address(users.dave.deleGator); + assertEq(erc20.allowance(delegator, other_), 0); + bytes memory executionCallData_ = _encodeSingle(address(erc20), 0, _approveCallData(other_, 0)); + _expectRevertBeforeHook(_terms(PERMISSION_ALL), executionCallData_, "ApprovalRevocationEnforcer:no-approval-to-revoke"); + } + + function test_erc20_revertWhenAllowanceCallFails() public { + // Target is a contract with no `allowance(address,address)` function; the high-level call reverts with + // empty returndata when ABI-decoding the (empty) response fails. + bytes memory executionCallData_ = _encodeSingle(address(enforcer), 0, _approveCallData(spender, 0)); + vm.prank(address(delegationManager)); + vm.expectRevert(); + enforcer.beforeHook(_terms(PERMISSION_ALL), hex"", singleDefaultMode, executionCallData_, bytes32(0), delegator, address(0)); + } + + ////////////////////////////// Valid cases (ERC-721 approve) ////////////////////////////// + + function test_erc721_revokeSucceedsForExistingApproval() public { + bytes memory executionCallData_ = _encodeSingle(address(erc721), 0, _approveCallData(address(0), mintedTokenId)); + _callBeforeHook(_terms(PERMISSION_ALL), executionCallData_); + } + + ////////////////////////////// Invalid cases (ERC-721 approve) ////////////////////////////// + + function test_erc721_revertWhenNoApproval() public { + // Mint a fresh token without approving it. + vm.prank(delegator); + erc721.mint(delegator); + uint256 freshTokenId_ = 1; + assertEq(erc721.getApproved(freshTokenId_), address(0)); + + bytes memory executionCallData_ = _encodeSingle(address(erc721), 0, _approveCallData(address(0), freshTokenId_)); + _expectRevertBeforeHook(_terms(PERMISSION_ALL), executionCallData_, "ApprovalRevocationEnforcer:no-approval-to-revoke"); + } + + function test_erc721_revertWhenGetApprovedCallFails() public { + // Non-existent token id reverts in OpenZeppelin's getApproved; the custom error bubbles up through the + // high-level call. + bytes memory executionCallData_ = _encodeSingle(address(erc721), 0, _approveCallData(address(0), 9999)); + vm.prank(address(delegationManager)); + vm.expectRevert(abi.encodeWithSignature("ERC721NonexistentToken(uint256)", 9999)); + enforcer.beforeHook(_terms(PERMISSION_ALL), hex"", singleDefaultMode, executionCallData_, bytes32(0), delegator, address(0)); + } + + ////////////////////////////// Valid cases (setApprovalForAll) ////////////////////////////// + + function test_setApprovalForAll_erc721_revokeSucceeds() public { + assertTrue(erc721.isApprovedForAll(delegator, operator)); + bytes memory executionCallData_ = _encodeSingle(address(erc721), 0, _setApprovalForAllCallData(operator, false)); + _callBeforeHook(_terms(PERMISSION_ALL), executionCallData_); + } + + function test_setApprovalForAll_erc1155_revokeSucceeds() public { + assertTrue(erc1155.isApprovedForAll(delegator, operator)); + bytes memory executionCallData_ = _encodeSingle(address(erc1155), 0, _setApprovalForAllCallData(operator, false)); + _callBeforeHook(_terms(PERMISSION_ALL), executionCallData_); + } + + ////////////////////////////// Invalid cases (setApprovalForAll) ////////////////////////////// + + function test_setApprovalForAll_revertWhenSettingTrue() public { + bytes memory executionCallData_ = _encodeSingle(address(erc721), 0, _setApprovalForAllCallData(operator, true)); + _expectRevertBeforeHook(_terms(PERMISSION_ALL), executionCallData_, "ApprovalRevocationEnforcer:not-a-revocation"); + } + + function test_setApprovalForAll_revertWhenNotApproved() public { + address other_ = address(users.dave.deleGator); + assertFalse(erc721.isApprovedForAll(delegator, other_)); + bytes memory executionCallData_ = _encodeSingle(address(erc721), 0, _setApprovalForAllCallData(other_, false)); + _expectRevertBeforeHook(_terms(PERMISSION_ALL), executionCallData_, "ApprovalRevocationEnforcer:no-approval-to-revoke"); + } + + function test_setApprovalForAll_revertWhenIsApprovedForAllCallFails() public { + // Target is a contract with no `isApprovedForAll(address,address)` function; the high-level call reverts + // with empty returndata when ABI-decoding the (empty) response fails. + bytes memory executionCallData_ = _encodeSingle(address(enforcer), 0, _setApprovalForAllCallData(operator, false)); + vm.prank(address(delegationManager)); + vm.expectRevert(); + enforcer.beforeHook(_terms(PERMISSION_ALL), hex"", singleDefaultMode, executionCallData_, bytes32(0), delegator, address(0)); + } + + ////////////////////////////// Per-flag gating (Permit2 approve) ////////////////////////////// + + function test_terms_onlyPermit2Approve_allowsPermit2Approve() public { + bytes memory executionCallData_ = _encodeSingle(PERMIT2, 0, _permit2ApproveCallData(address(erc20), spender, 0, 0)); + _callBeforeHook(_terms(PERMISSION_PERMIT2_APPROVE), executionCallData_); + } + + function test_terms_onlyPermit2Approve_blocksErc20() public { + bytes memory executionCallData_ = _encodeSingle(address(erc20), 0, _approveCallData(spender, 0)); + _expectRevertBeforeHook(_terms(PERMISSION_PERMIT2_APPROVE), executionCallData_, "ApprovalRevocationEnforcer:erc20-approve-not-allowed"); + } + + function test_terms_onlyPermit2Approve_blocksErc721Approve() public { + bytes memory executionCallData_ = _encodeSingle(address(erc721), 0, _approveCallData(address(0), mintedTokenId)); + _expectRevertBeforeHook(_terms(PERMISSION_PERMIT2_APPROVE), executionCallData_, "ApprovalRevocationEnforcer:erc721-approve-not-allowed"); + } + + function test_terms_onlyPermit2Approve_blocksSetApprovalForAll() public { + bytes memory executionCallData_ = _encodeSingle(address(erc721), 0, _setApprovalForAllCallData(operator, false)); + _expectRevertBeforeHook(_terms(PERMISSION_PERMIT2_APPROVE), executionCallData_, "ApprovalRevocationEnforcer:setApprovalForAll-not-allowed"); + } + + function test_terms_withoutPermit2Approve_blocksPermit2Approve() public { + uint8 flags_ = PERMISSION_ERC20_APPROVE | PERMISSION_ERC721_APPROVE | PERMISSION_SET_APPROVAL_FOR_ALL; + bytes memory executionCallData_ = _encodeSingle(PERMIT2, 0, _permit2ApproveCallData(address(erc20), spender, 0, 0)); + _expectRevertBeforeHook(_terms(flags_), executionCallData_, "ApprovalRevocationEnforcer:permit2-approve-not-allowed"); + } + + ////////////////////////////// Valid cases (Permit2 approve) ////////////////////////////// + + function test_permit2_revokeSucceeds() public { + bytes memory executionCallData_ = _encodeSingle(PERMIT2, 0, _permit2ApproveCallData(address(erc20), spender, 0, 0)); + _callBeforeHook(_terms(PERMISSION_ALL), executionCallData_); + } + + function test_permit2_revokeSucceedsWithArbitraryTokenAndSpender() public { + // The enforcer does not constrain the (token, spender) pair on its own — those should be pinned via + // composition (e.g. AllowedCalldataEnforcer). Here we just verify the hook accepts arbitrary values. + bytes memory executionCallData_ = + _encodeSingle(PERMIT2, 0, _permit2ApproveCallData(address(0xdead), address(0xbeef), 0, 0)); + _callBeforeHook(_terms(PERMISSION_ALL), executionCallData_); + } + + ////////////////////////////// Invalid cases (Permit2 approve) ////////////////////////////// + + function test_permit2_revertOnNonPermit2Target() public { + bytes memory executionCallData_ = _encodeSingle(address(erc20), 0, _permit2ApproveCallData(address(erc20), spender, 0, 0)); + _expectRevertBeforeHook(_terms(PERMISSION_ALL), executionCallData_, "ApprovalRevocationEnforcer:invalid-permit2-target"); + } + + function test_permit2_revertOnNonZeroAmount() public { + bytes memory executionCallData_ = _encodeSingle(PERMIT2, 0, _permit2ApproveCallData(address(erc20), spender, 1, 0)); + _expectRevertBeforeHook(_terms(PERMISSION_ALL), executionCallData_, "ApprovalRevocationEnforcer:non-zero-amount"); + } + + function test_permit2_revertOnNonZeroExpiration() public { + bytes memory executionCallData_ = _encodeSingle(PERMIT2, 0, _permit2ApproveCallData(address(erc20), spender, 0, 1)); + _expectRevertBeforeHook(_terms(PERMISSION_ALL), executionCallData_, "ApprovalRevocationEnforcer:non-zero-expiration"); + } + + function test_permit2_revertOnMaxNonZeroAmount() public { + bytes memory executionCallData_ = _encodeSingle(PERMIT2, 0, _permit2ApproveCallData(address(erc20), spender, type(uint160).max, 0)); + _expectRevertBeforeHook(_terms(PERMISSION_ALL), executionCallData_, "ApprovalRevocationEnforcer:non-zero-amount"); + } + + function test_permit2_revertOnMaxNonZeroExpiration() public { + bytes memory executionCallData_ = _encodeSingle(PERMIT2, 0, _permit2ApproveCallData(address(erc20), spender, 0, type(uint48).max)); + _expectRevertBeforeHook(_terms(PERMISSION_ALL), executionCallData_, "ApprovalRevocationEnforcer:non-zero-expiration"); + } + + function test_permit2_revertOnTruncatedCallData() public { + // 4 selector + 3 words = 100 bytes; matches Permit2 selector dispatch but fails the length gate. + bytes memory truncated_ = abi.encodePacked( + PERMIT2_APPROVE_SELECTOR, bytes32(uint256(uint160(address(erc20)))), bytes32(uint256(uint160(spender))), bytes32(uint256(0)) + ); + bytes memory executionCallData_ = _encodeSingle(PERMIT2, 0, truncated_); + _expectRevertBeforeHook(_terms(PERMISSION_ALL), executionCallData_, "ApprovalRevocationEnforcer:permit2-invalid-execution-length"); + } + + function test_permit2_revertOnExtraTrailingByte() public { + bytes memory longCallData_ = abi.encodePacked(_permit2ApproveCallData(address(erc20), spender, 0, 0), bytes1(0x00)); + bytes memory executionCallData_ = _encodeSingle(PERMIT2, 0, longCallData_); + _expectRevertBeforeHook(_terms(PERMISSION_ALL), executionCallData_, "ApprovalRevocationEnforcer:permit2-invalid-execution-length"); + } + + function test_permit2_revertOnNonZeroValue() public { + bytes memory executionCallData_ = _encodeSingle(PERMIT2, 1, _permit2ApproveCallData(address(erc20), spender, 0, 0)); + _expectRevertBeforeHook(_terms(PERMISSION_ALL), executionCallData_, "ApprovalRevocationEnforcer:invalid-value"); + } + + ////////////////////////////// Per-flag gating (Permit2 lockdown) ////////////////////////////// + + function test_terms_onlyPermit2Lockdown_allowsLockdown() public { + bytes memory executionCallData_ = + _encodeSingle(PERMIT2, 0, _permit2LockdownCallData(_singlePair(address(erc20), spender))); + _callBeforeHook(_terms(PERMISSION_PERMIT2_LOCKDOWN), executionCallData_); + } + + function test_terms_onlyPermit2Lockdown_blocksErc20() public { + bytes memory executionCallData_ = _encodeSingle(address(erc20), 0, _approveCallData(spender, 0)); + _expectRevertBeforeHook(_terms(PERMISSION_PERMIT2_LOCKDOWN), executionCallData_, "ApprovalRevocationEnforcer:erc20-approve-not-allowed"); + } + + function test_terms_onlyPermit2Lockdown_blocksErc721Approve() public { + bytes memory executionCallData_ = _encodeSingle(address(erc721), 0, _approveCallData(address(0), mintedTokenId)); + _expectRevertBeforeHook(_terms(PERMISSION_PERMIT2_LOCKDOWN), executionCallData_, "ApprovalRevocationEnforcer:erc721-approve-not-allowed"); + } + + function test_terms_onlyPermit2Lockdown_blocksSetApprovalForAll() public { + bytes memory executionCallData_ = _encodeSingle(address(erc721), 0, _setApprovalForAllCallData(operator, false)); + _expectRevertBeforeHook(_terms(PERMISSION_PERMIT2_LOCKDOWN), executionCallData_, "ApprovalRevocationEnforcer:setApprovalForAll-not-allowed"); + } + + function test_terms_onlyPermit2Lockdown_blocksPermit2Approve() public { + bytes memory executionCallData_ = _encodeSingle(PERMIT2, 0, _permit2ApproveCallData(address(erc20), spender, 0, 0)); + _expectRevertBeforeHook(_terms(PERMISSION_PERMIT2_LOCKDOWN), executionCallData_, "ApprovalRevocationEnforcer:permit2-approve-not-allowed"); + } + + function test_terms_onlyPermit2Approve_blocksLockdown() public { + bytes memory executionCallData_ = + _encodeSingle(PERMIT2, 0, _permit2LockdownCallData(_singlePair(address(erc20), spender))); + _expectRevertBeforeHook(_terms(PERMISSION_PERMIT2_APPROVE), executionCallData_, "ApprovalRevocationEnforcer:permit2-lockdown-not-allowed"); + } + + function test_terms_withoutPermit2Lockdown_blocksLockdown() public { + uint8 flags_ = PERMISSION_ERC20_APPROVE | PERMISSION_ERC721_APPROVE | PERMISSION_SET_APPROVAL_FOR_ALL | PERMISSION_PERMIT2_APPROVE; + bytes memory executionCallData_ = + _encodeSingle(PERMIT2, 0, _permit2LockdownCallData(_singlePair(address(erc20), spender))); + _expectRevertBeforeHook(_terms(flags_), executionCallData_, "ApprovalRevocationEnforcer:permit2-lockdown-not-allowed"); + } + + ////////////////////////////// Valid cases (Permit2 lockdown) ////////////////////////////// + + function test_permit2Lockdown_revokeSucceedsForSinglePair() public { + bytes memory executionCallData_ = + _encodeSingle(PERMIT2, 0, _permit2LockdownCallData(_singlePair(address(erc20), spender))); + _callBeforeHook(_terms(PERMISSION_ALL), executionCallData_); + } + + function test_permit2Lockdown_revokeSucceedsForMultiplePairs() public { + TokenSpenderPair[] memory pairs_ = new TokenSpenderPair[](3); + pairs_[0] = TokenSpenderPair({ token: address(erc20), spender: spender }); + pairs_[1] = TokenSpenderPair({ token: address(0xdead), spender: address(0xbeef) }); + pairs_[2] = TokenSpenderPair({ token: address(erc721), spender: operator }); + bytes memory executionCallData_ = _encodeSingle(PERMIT2, 0, _permit2LockdownCallData(pairs_)); + _callBeforeHook(_terms(PERMISSION_ALL), executionCallData_); + } + + function test_permit2Lockdown_revokeSucceedsForEmptyArray() public { + // Empty lockdown is structurally a no-op — Permit2 accepts it. The enforcer accepts it too: there is + // nothing here that could ever grant authority. Pinned as documented behavior so future refactors don't + // silently change it. + bytes memory executionCallData_ = _encodeSingle(PERMIT2, 0, _permit2LockdownCallData(new TokenSpenderPair[](0))); + _callBeforeHook(_terms(PERMISSION_ALL), executionCallData_); + } + + ////////////////////////////// Invalid cases (Permit2 lockdown) ////////////////////////////// + + function test_permit2Lockdown_revertOnNonPermit2Target() public { + bytes memory executionCallData_ = + _encodeSingle(address(erc20), 0, _permit2LockdownCallData(_singlePair(address(erc20), spender))); + _expectRevertBeforeHook(_terms(PERMISSION_ALL), executionCallData_, "ApprovalRevocationEnforcer:invalid-permit2-target"); + } + + function test_permit2Lockdown_revertOnNonZeroValue() public { + bytes memory executionCallData_ = + _encodeSingle(PERMIT2, 1, _permit2LockdownCallData(_singlePair(address(erc20), spender))); + _expectRevertBeforeHook(_terms(PERMISSION_ALL), executionCallData_, "ApprovalRevocationEnforcer:invalid-value"); + } + + function test_permit2Lockdown_acceptsMalformedPayload_safetyRestsOnPermit2() public { + // The lockdown branch performs no calldata-shape validation: the structural argument is that any contract + // at the canonical Permit2 address can only zero allowance amounts under this selector. Pin the + // enforcer-level behavior here so future refactors don't silently introduce a length check that breaks + // composition with `ExactCalldataEnforcer` for non-standard pinning shapes. + bytes memory malformed_ = abi.encodePacked(PERMIT2_LOCKDOWN_SELECTOR, hex"deadbeef"); + bytes memory executionCallData_ = _encodeSingle(PERMIT2, 0, malformed_); + _callBeforeHook(_terms(PERMISSION_ALL), executionCallData_); + } + + ////////////////////////////// Per-flag gating (Permit2 invalidateNonces) ////////////////////////////// + + function test_terms_onlyPermit2InvalidateNonces_allowsInvalidateNonces() public { + bytes memory executionCallData_ = _encodeSingle(PERMIT2, 0, _permit2InvalidateNoncesCallData(address(erc20), spender, 1)); + _callBeforeHook(_terms(PERMISSION_PERMIT2_INVALIDATE_NONCES), executionCallData_); + } + + function test_terms_onlyPermit2InvalidateNonces_blocksErc20() public { + bytes memory executionCallData_ = _encodeSingle(address(erc20), 0, _approveCallData(spender, 0)); + _expectRevertBeforeHook(_terms(PERMISSION_PERMIT2_INVALIDATE_NONCES), executionCallData_, "ApprovalRevocationEnforcer:erc20-approve-not-allowed"); + } + + function test_terms_onlyPermit2InvalidateNonces_blocksErc721Approve() public { + bytes memory executionCallData_ = _encodeSingle(address(erc721), 0, _approveCallData(address(0), mintedTokenId)); + _expectRevertBeforeHook(_terms(PERMISSION_PERMIT2_INVALIDATE_NONCES), executionCallData_, "ApprovalRevocationEnforcer:erc721-approve-not-allowed"); + } + + function test_terms_onlyPermit2InvalidateNonces_blocksSetApprovalForAll() public { + bytes memory executionCallData_ = _encodeSingle(address(erc721), 0, _setApprovalForAllCallData(operator, false)); + _expectRevertBeforeHook(_terms(PERMISSION_PERMIT2_INVALIDATE_NONCES), executionCallData_, "ApprovalRevocationEnforcer:setApprovalForAll-not-allowed"); + } + + function test_terms_onlyPermit2InvalidateNonces_blocksPermit2Approve() public { + bytes memory executionCallData_ = _encodeSingle(PERMIT2, 0, _permit2ApproveCallData(address(erc20), spender, 0, 0)); + _expectRevertBeforeHook(_terms(PERMISSION_PERMIT2_INVALIDATE_NONCES), executionCallData_, "ApprovalRevocationEnforcer:permit2-approve-not-allowed"); + } + + function test_terms_onlyPermit2InvalidateNonces_blocksLockdown() public { + bytes memory executionCallData_ = + _encodeSingle(PERMIT2, 0, _permit2LockdownCallData(_singlePair(address(erc20), spender))); + _expectRevertBeforeHook(_terms(PERMISSION_PERMIT2_INVALIDATE_NONCES), executionCallData_, "ApprovalRevocationEnforcer:permit2-lockdown-not-allowed"); + } + + function test_terms_withoutPermit2InvalidateNonces_blocksInvalidateNonces() public { + uint8 flags_ = PERMISSION_ERC20_APPROVE | PERMISSION_ERC721_APPROVE | PERMISSION_SET_APPROVAL_FOR_ALL + | PERMISSION_PERMIT2_APPROVE | PERMISSION_PERMIT2_LOCKDOWN; + bytes memory executionCallData_ = _encodeSingle(PERMIT2, 0, _permit2InvalidateNoncesCallData(address(erc20), spender, 1)); + _expectRevertBeforeHook(_terms(flags_), executionCallData_, "ApprovalRevocationEnforcer:permit2-invalidate-nonces-not-allowed"); + } + + ////////////////////////////// Valid cases (Permit2 invalidateNonces) ////////////////////////////// + + function test_permit2InvalidateNonces_succeedsWithArbitraryNonce() public { + // The enforcer does not validate the nonce value — Permit2 itself enforces strict monotonicity and the + // per-call uint16-bounded delta. Pin the enforcer-level acceptance with a representative nonce. + bytes memory executionCallData_ = _encodeSingle(PERMIT2, 0, _permit2InvalidateNoncesCallData(address(erc20), spender, 1)); + _callBeforeHook(_terms(PERMISSION_ALL), executionCallData_); + } + + function test_permit2InvalidateNonces_succeedsWithMaxNonce() public { + bytes memory executionCallData_ = + _encodeSingle(PERMIT2, 0, _permit2InvalidateNoncesCallData(address(erc20), spender, type(uint48).max)); + _callBeforeHook(_terms(PERMISSION_ALL), executionCallData_); + } + + function test_permit2InvalidateNonces_succeedsWithArbitraryTokenAndSpender() public { + bytes memory executionCallData_ = + _encodeSingle(PERMIT2, 0, _permit2InvalidateNoncesCallData(address(0xdead), address(0xbeef), 7)); + _callBeforeHook(_terms(PERMISSION_ALL), executionCallData_); + } + + function test_permit2InvalidateNonces_acceptsMalformedPayload_safetyRestsOnPermit2() public { + // Same rationale as the lockdown counterpart: nonce monotonicity is enforced inside Permit2, so the + // enforcer does not validate calldata shape beyond the selector + target. + bytes memory malformed_ = abi.encodePacked(PERMIT2_INVALIDATE_NONCES_SELECTOR, hex"deadbeef"); + bytes memory executionCallData_ = _encodeSingle(PERMIT2, 0, malformed_); + _callBeforeHook(_terms(PERMISSION_ALL), executionCallData_); + } + + ////////////////////////////// Invalid cases (Permit2 invalidateNonces) ////////////////////////////// + + function test_permit2InvalidateNonces_revertOnNonPermit2Target() public { + bytes memory executionCallData_ = + _encodeSingle(address(erc20), 0, _permit2InvalidateNoncesCallData(address(erc20), spender, 1)); + _expectRevertBeforeHook(_terms(PERMISSION_ALL), executionCallData_, "ApprovalRevocationEnforcer:invalid-permit2-target"); + } + + function test_permit2InvalidateNonces_revertOnNonZeroValue() public { + bytes memory executionCallData_ = + _encodeSingle(PERMIT2, 1, _permit2InvalidateNoncesCallData(address(erc20), spender, 1)); + _expectRevertBeforeHook(_terms(PERMISSION_ALL), executionCallData_, "ApprovalRevocationEnforcer:invalid-value"); + } + + ////////////////////////////// Generic invalid cases ////////////////////////////// + + function test_revertOnNonZeroValue() public { + bytes memory executionCallData_ = _encodeSingle(address(erc20), 1, _approveCallData(spender, 0)); + _expectRevertBeforeHook(_terms(PERMISSION_ALL), executionCallData_, "ApprovalRevocationEnforcer:invalid-value"); + } + + function test_revertOnInvalidExecutionLengthShort() public { + bytes memory shortCallData_ = abi.encodePacked(IERC20.approve.selector, bytes32(uint256(uint160(spender)))); + bytes memory executionCallData_ = _encodeSingle(address(erc20), 0, shortCallData_); + _expectRevertBeforeHook(_terms(PERMISSION_ALL), executionCallData_, "ApprovalRevocationEnforcer:invalid-execution-length"); + } + + function test_revertOnInvalidExecutionLengthLong() public { + bytes memory longCallData_ = abi.encodePacked(_approveCallData(spender, 0), bytes1(0x00)); + bytes memory executionCallData_ = _encodeSingle(address(erc20), 0, longCallData_); + _expectRevertBeforeHook(_terms(PERMISSION_ALL), executionCallData_, "ApprovalRevocationEnforcer:invalid-execution-length"); + } + + function test_revertOnInvalidMethod() public { + bytes memory wrongMethodCallData_ = abi.encodeWithSelector(IERC20.transfer.selector, spender, uint256(0)); + bytes memory executionCallData_ = _encodeSingle(address(erc20), 0, wrongMethodCallData_); + _expectRevertBeforeHook(_terms(PERMISSION_ALL), executionCallData_, "ApprovalRevocationEnforcer:invalid-method"); + } + + function test_revertWithInvalidCallTypeMode() public { + bytes memory executionCallData_ = ExecutionLib.encodeBatch(new Execution[](2)); + vm.expectRevert("CaveatEnforcer:invalid-call-type"); + enforcer.beforeHook(_terms(PERMISSION_ALL), hex"", batchDefaultMode, executionCallData_, bytes32(0), delegator, address(0)); + } + + function test_revertWithInvalidExecutionMode() public { + vm.expectRevert("CaveatEnforcer:invalid-execution-type"); + enforcer.beforeHook(_terms(PERMISSION_ALL), hex"", singleTryMode, hex"", bytes32(0), delegator, address(0)); + } + + ////////////////////////////// Integration ////////////////////////////// + + function test_integration_revokesErc20Allowance() public { + assertEq(erc20.allowance(delegator, spender), 42 ether); + + Execution memory execution_ = + Execution({ target: address(erc20), value: 0, callData: _approveCallData(spender, 0) }); + + Caveat[] memory caveats_ = new Caveat[](1); + caveats_[0] = Caveat({ args: hex"", enforcer: address(enforcer), terms: _terms(PERMISSION_ALL) }); + Delegation memory delegation_ = Delegation({ + delegate: address(users.bob.deleGator), + delegator: delegator, + authority: ROOT_AUTHORITY, + caveats: caveats_, + salt: 0, + signature: hex"" + }); + delegation_ = signDelegation(users.alice, delegation_); + + Delegation[] memory delegations_ = new Delegation[](1); + delegations_[0] = delegation_; + + invokeDelegation_UserOp(users.bob, delegations_, execution_); + assertEq(erc20.allowance(delegator, spender), 0); + } + + function test_integration_revokesErc721Approval() public { + assertEq(erc721.getApproved(mintedTokenId), spender); + + Execution memory execution_ = + Execution({ target: address(erc721), value: 0, callData: _approveCallData(address(0), mintedTokenId) }); + + Caveat[] memory caveats_ = new Caveat[](1); + caveats_[0] = Caveat({ args: hex"", enforcer: address(enforcer), terms: _terms(PERMISSION_ALL) }); + Delegation memory delegation_ = Delegation({ + delegate: address(users.bob.deleGator), + delegator: delegator, + authority: ROOT_AUTHORITY, + caveats: caveats_, + salt: 0, + signature: hex"" + }); + delegation_ = signDelegation(users.alice, delegation_); + + Delegation[] memory delegations_ = new Delegation[](1); + delegations_[0] = delegation_; + + invokeDelegation_UserOp(users.bob, delegations_, execution_); + assertEq(erc721.getApproved(mintedTokenId), address(0)); + } + + function test_integration_revokesSetApprovalForAll() public { + assertTrue(erc1155.isApprovedForAll(delegator, operator)); + + Execution memory execution_ = + Execution({ target: address(erc1155), value: 0, callData: _setApprovalForAllCallData(operator, false) }); + + Caveat[] memory caveats_ = new Caveat[](1); + caveats_[0] = Caveat({ args: hex"", enforcer: address(enforcer), terms: _terms(PERMISSION_ALL) }); + Delegation memory delegation_ = Delegation({ + delegate: address(users.bob.deleGator), + delegator: delegator, + authority: ROOT_AUTHORITY, + caveats: caveats_, + salt: 0, + signature: hex"" + }); + delegation_ = signDelegation(users.alice, delegation_); + + Delegation[] memory delegations_ = new Delegation[](1); + delegations_[0] = delegation_; + + invokeDelegation_UserOp(users.bob, delegations_, execution_); + assertFalse(erc1155.isApprovedForAll(delegator, operator)); + } + + function test_integration_onlyErc20_revokesErc20AllowanceAndBlocksOtherPrimitives() public { + assertEq(erc20.allowance(delegator, spender), 42 ether); + + Caveat[] memory caveats_ = new Caveat[](1); + caveats_[0] = Caveat({ args: hex"", enforcer: address(enforcer), terms: _terms(PERMISSION_ERC20_APPROVE) }); + Delegation memory delegation_ = Delegation({ + delegate: address(users.bob.deleGator), + delegator: delegator, + authority: ROOT_AUTHORITY, + caveats: caveats_, + salt: 0, + signature: hex"" + }); + delegation_ = signDelegation(users.alice, delegation_); + + Delegation[] memory delegations_ = new Delegation[](1); + delegations_[0] = delegation_; + + // ERC-20 revocation succeeds. + Execution memory erc20Execution_ = Execution({ target: address(erc20), value: 0, callData: _approveCallData(spender, 0) }); + invokeDelegation_UserOp(users.bob, delegations_, erc20Execution_); + assertEq(erc20.allowance(delegator, spender), 0); + + // ERC-721 approve revocation is blocked (UserOp swallows revert; approval unchanged). + Execution memory erc721Execution_ = Execution({ target: address(erc721), value: 0, callData: _approveCallData(address(0), mintedTokenId) }); + invokeDelegation_UserOp(users.bob, delegations_, erc721Execution_); + assertEq(erc721.getApproved(mintedTokenId), spender); + } + + ////////////////////////////// Redelegation ////////////////////////////// + + /** + * @notice Alice -> Bob -> Carol, with the `ApprovalRevocationEnforcer` caveat on Alice's (root) link. Carol + * redeems. The caveat's `beforeHook` receives `_delegator = Alice`, matching the account whose approval is + * actually cleared at execution time. Works end-to-end. + */ + function test_integration_redelegation_caveatOnRootLink_revokesRootAllowance() public { + assertEq(erc20.allowance(delegator, spender), 42 ether); + + Caveat[] memory caveats_ = new Caveat[](1); + caveats_[0] = Caveat({ args: hex"", enforcer: address(enforcer), terms: _terms(PERMISSION_ALL) }); + Delegation memory aliceDelegation_ = Delegation({ + delegate: address(users.bob.deleGator), + delegator: delegator, + authority: ROOT_AUTHORITY, + caveats: caveats_, + salt: 0, + signature: hex"" + }); + aliceDelegation_ = signDelegation(users.alice, aliceDelegation_); + bytes32 aliceDelegationHash_ = EncoderLib._getDelegationHash(aliceDelegation_); + + Delegation memory bobDelegation_ = Delegation({ + delegate: address(users.carol.deleGator), + delegator: address(users.bob.deleGator), + authority: aliceDelegationHash_, + caveats: new Caveat[](0), + salt: 0, + signature: hex"" + }); + bobDelegation_ = signDelegation(users.bob, bobDelegation_); + + Delegation[] memory delegations_ = new Delegation[](2); + delegations_[0] = bobDelegation_; + delegations_[1] = aliceDelegation_; + + Execution memory execution_ = + Execution({ target: address(erc20), value: 0, callData: _approveCallData(spender, 0) }); + + invokeDelegation_UserOp(users.carol, delegations_, execution_); + assertEq(erc20.allowance(delegator, spender), 0); + } + + /** + * @notice Alice -> Bob -> Carol, with the caveat on Bob's (intermediate) link. The `beforeHook` runs with + * `_delegator = Bob`, so the pre-check queries `allowance(Bob, spender)`. Bob has no such allowance, so the + * hook reverts even though Alice (the root, whose account actually runs `approve`) does have one. + * + * @dev This test pins down a subtlety of redelegation semantics: caveats are evaluated against the delegator + * of their own link, not the root of the chain. For this enforcer it means an intermediate-link caveat + * checks the *intermediate* delegator's approval state, which is almost never what the delegator intends. + */ + function test_integration_redelegation_caveatOnIntermediateLink_revertsWhenIntermediateHasNoApproval() public { + assertEq(erc20.allowance(delegator, spender), 42 ether); + assertEq(erc20.allowance(address(users.bob.deleGator), spender), 0); + + Delegation memory aliceDelegation_ = Delegation({ + delegate: address(users.bob.deleGator), + delegator: delegator, + authority: ROOT_AUTHORITY, + caveats: new Caveat[](0), + salt: 0, + signature: hex"" + }); + aliceDelegation_ = signDelegation(users.alice, aliceDelegation_); + bytes32 aliceDelegationHash_ = EncoderLib._getDelegationHash(aliceDelegation_); + + Caveat[] memory caveats_ = new Caveat[](1); + caveats_[0] = Caveat({ args: hex"", enforcer: address(enforcer), terms: _terms(PERMISSION_ALL) }); + Delegation memory bobDelegation_ = Delegation({ + delegate: address(users.carol.deleGator), + delegator: address(users.bob.deleGator), + authority: aliceDelegationHash_, + caveats: caveats_, + salt: 0, + signature: hex"" + }); + bobDelegation_ = signDelegation(users.bob, bobDelegation_); + + Delegation[] memory delegations_ = new Delegation[](2); + delegations_[0] = bobDelegation_; + delegations_[1] = aliceDelegation_; + + Execution memory execution_ = + Execution({ target: address(erc20), value: 0, callData: _approveCallData(spender, 0) }); + + // UserOp swallows the enforcer revert; the effect is that the approval is NOT cleared. + invokeDelegation_UserOp(users.carol, delegations_, execution_); + assertEq(erc20.allowance(delegator, spender), 42 ether); + } + + /** + * @notice Unit-level check on the link-local `_delegator` semantics. The hook queries the external token + * using whatever address is passed as `_delegator`; it does not reach back into the chain to find the root. + */ + function test_unit_beforeHook_usesPassedDelegatorNotRoot() public { + address intermediate_ = address(users.bob.deleGator); + assertEq(erc20.allowance(intermediate_, spender), 0); + + bytes memory executionCallData_ = _encodeSingle(address(erc20), 0, _approveCallData(spender, 0)); + + vm.prank(address(delegationManager)); + vm.expectRevert("ApprovalRevocationEnforcer:no-approval-to-revoke"); + enforcer.beforeHook(_terms(PERMISSION_ALL), hex"", singleDefaultMode, executionCallData_, bytes32(0), intermediate_, address(0)); + + // And once Bob has an allowance of his own, the pre-check passes against Bob's state (regardless of who + // would actually execute the call). + vm.prank(intermediate_); + erc20.approve(spender, 1); + vm.prank(address(delegationManager)); + enforcer.beforeHook(_terms(PERMISSION_ALL), hex"", singleDefaultMode, executionCallData_, bytes32(0), intermediate_, address(0)); + } + + ////////////////////////////// Additional coverage ////////////////////////////// + + /** + * @notice `approve(non-zero, 0)` targeting an ERC-721 contract routes to the ERC-20 branch (because the + * first parameter is non-zero). The pre-check calls `allowance(delegator, spender)` on the ERC-721, which + * does not implement it and therefore reverts (empty returndata after ABI-decode). + */ + function test_crossStandard_erc721TargetWithErc20Style_reverts() public { + bytes memory executionCallData_ = _encodeSingle(address(erc721), 0, _approveCallData(spender, 0)); + vm.prank(address(delegationManager)); + vm.expectRevert(); + enforcer.beforeHook(_terms(PERMISSION_ALL), hex"", singleDefaultMode, executionCallData_, bytes32(0), delegator, address(0)); + } + + /** + * @notice `approve(address(0), 0)` on an ERC-20 is routed to the ERC-721 branch by the `firstParam == 0` + * heuristic. The branch then calls `getApproved(0)` on the target. Standard ERC-20s do not implement + * `getApproved`, so the pre-check reverts. Pins the behavior of this edge case so future refactors don't + * silently change routing. + */ + function test_edgeCase_approveAddressZeroAmountZeroOnErc20_reverts() public { + bytes memory executionCallData_ = _encodeSingle(address(erc20), 0, _approveCallData(address(0), 0)); + vm.prank(address(delegationManager)); + vm.expectRevert(); + enforcer.beforeHook(_terms(PERMISSION_ALL), hex"", singleDefaultMode, executionCallData_, bytes32(0), delegator, address(0)); + } + + function _getEnforcer() internal view override returns (ICaveatEnforcer) { + return ICaveatEnforcer(address(enforcer)); + } +}