From f7804036646235b1b1d928f67a6c3a0650b6c8ed Mon Sep 17 00:00:00 2001 From: highskore Date: Wed, 10 Jun 2026 17:38:43 +0200 Subject: [PATCH] fix(security): confused-deputy deny + reentrancy guard + tree acyclicity + config-id split (F1/F2/G/I) Ported from rhinestonewtf/daimon#186. F1 (confused-deputy deny): EnforcementLib.enforceAction now default-denies a MANDATE call whose `to` is an installed ROOT validator (RootStorageLib) or one of THIS mandate's own policy sigils. Adds a per-mandate `mandateSigils` reverse index (MandateStorageLib, ERC-7201 append-only) populated via _recordSigil in _registerMandate and cleared in _clearMandateSets. F2 (reentrancy): Daimon inherits solady ReentrancyGuardTransient; executeWithSig is now nonReentrant (existing deadline signature preserved). G (tree acyclicity): OmniSigilTreeLib.validateExpressionTree requires each child index to be strictly less than its parent's (new NodeChildIndexNotDescending error), guaranteeing a DAG and bounding evaluateNode recursion. I (config-id split): IdLib.toMandateConfigId renamed to toOutcomeConfigId; new toSignatureConfigId ("daimon.signature") domain-separates the 1271 tier from the outcome tier. Callers updated (MandateEngine, EnforcementLib, tests). Plus natspec-only clarifications across SpendSigil (tumbling-window + ERC-777 residual), NativeValueLimitSigil (per-call), RateLimitConfigLib (uint32 ~2106 wrap), TimeFrameSigil (fail-open + time-only composition), AttestationSigil (no cumulative cap), OmniSigilTreeLib.fill (usage reset on re-bind), IdLib (per-mandate cap non-aggregation). D (subMode/mandateId in exec digest) intentionally excluded: reverted upstream as an invalid finding. This repo's ERC-1608 deadline + Execute digest design is left untouched. Co-Authored-By: Claude Fable 5 --- src/Daimon.sol | 8 +++ src/core/MandateEngine.sol | 56 +++++++++++++++---- src/lib/EnforcementLib.sol | 19 ++++++- src/lib/IdLib.sol | 19 ++++++- src/lib/MandateStorageLib.sol | 26 +++++++-- .../AttestationSigil/AttestationSigil.sol | 5 +- .../NativeValueLimitSigil.sol | 6 +- src/sigils/OmniSigil/lib/OmniSigilTreeLib.sol | 31 ++++++++-- .../RateLimitSigil/lib/RateLimitConfigLib.sol | 4 ++ src/sigils/SpendSigil/SpendSigil.sol | 37 +++++++----- src/sigils/TimeFrameSigil/TimeFrameSigil.sol | 14 +++++ .../Daimon/spendSigil/spendSigil.t.sol | 2 +- .../swapWithApprove/swapWithApprove.t.sol | 2 +- 13 files changed, 187 insertions(+), 42 deletions(-) diff --git a/src/Daimon.sol b/src/Daimon.sol index 6df3a23..4a710fb 100644 --- a/src/Daimon.sol +++ b/src/Daimon.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.28; // Contracts import { Initializable } from "solady/utils/Initializable.sol"; import { UUPSUpgradeable } from "solady/utils/UUPSUpgradeable.sol"; +import { ReentrancyGuardTransient } from "solady/utils/ReentrancyGuardTransient.sol"; import { Receiver } from "solady/accounts/Receiver.sol"; import { ERC1271 } from "solady/accounts/ERC1271.sol"; import { DaimonERC7739 } from "@core/DaimonERC7739.sol"; @@ -50,6 +51,7 @@ import { Mandate, MandateId } from "@types/MandateTypes.sol"; contract Daimon is Initializable, UUPSUpgradeable, + ReentrancyGuardTransient, Receiver, DaimonERC7739, RootRegistry, @@ -183,6 +185,11 @@ contract Daimon is /// path enforces the mandate's sigils over *every* call. The signed digest commits to /// `(mode, executionData, nonce, deadline)`. Reverts on failure; the single-use nonce is burned first /// (CEI), but only AFTER the `deadline` check, so an expired payload cannot burn its nonce. + /// `nonReentrant` (solady transient guard): a nested `executeWithSig` would clobber an outer execution's + /// SpendSigil pre-execution balance snapshot — kept in transient storage keyed by `(account, token)`, not + /// per-execution — and zero out its balance-delta backstop, so nested entry is forbidden. The ROOT + /// self-call to `bindMandates`/`installRoot`/`upgrade` is a DIFFERENT function (not a nested + /// `executeWithSig`), so it is not blocked. function executeWithSig( bytes32 mode, bytes calldata executionData, @@ -193,6 +200,7 @@ contract Daimon is external payable virtual + nonReentrant returns (bytes[] memory results) { if (sig.length == 0) revert InvalidSignatureMode(0); diff --git a/src/core/MandateEngine.sol b/src/core/MandateEngine.sol index 6e101b7..3925f3d 100644 --- a/src/core/MandateEngine.sol +++ b/src/core/MandateEngine.sol @@ -300,16 +300,23 @@ abstract contract MandateEngine is IMandateEngine { revert UnsupportedSigil(sigil); } $.actionSigils[aid][pid].add(sigil); + // Record the sigil in the mandate's reverse index so the MANDATE path can default-deny any agent + // call to it — it holds this account's policy config keyed by the account + // (see {MandateStorageLib.mandateSigils}). + _recordSigil($, pid, sigil); IActionSigil(sigil) .initializeWithMultiplexer(address(this), cid, a.sigils[j].initData); } } // Register + initialize the per-execution outcome sigils and the per-mandate signature (ERC-1271) - // sigils, mirroring action-sigil wiring. Both share the per-mandate ConfigId (keyed by the mandate - // alone, not per-(target, selector)) — {IdLib.toMandateConfigId} is domain-separated from the - // per-action id, so the two categories never collide even when one address serves both roles. - ConfigId mcid = IdLib.toMandateConfigId(pid); + // sigils, mirroring action-sigil wiring. Each tier keys config by the mandate alone (not per-(target, + // selector)), but the OUTCOME tier and the SIGNATURE tier use DISTINCT, domain-separated ConfigIds + // ({IdLib.toOutcomeConfigId} vs {IdLib.toSignatureConfigId}) — and both are separated from the + // per-action id — so the three categories never collide, even for a single address that serves more + // than one tier (e.g. a future sigil implementing both {IOutcomeSigil} and {I1271Sigil}). + ConfigId ocid = IdLib.toOutcomeConfigId(pid); + ConfigId scid = IdLib.toSignatureConfigId(pid); for (uint256 i; i < s.outcomeSigils.length; ++i) { OutcomeSigilData memory o = s.outcomeSigils[i]; // Fail-closed at bind: an outcome sigil must advertise {IOutcomeSigil} via ERC-165. This rejects @@ -320,7 +327,8 @@ abstract contract MandateEngine is IMandateEngine { revert UnsupportedSigil(o.sigil); } $.outcomeSigils[pid].add(o.sigil); - IOutcomeSigil(o.sigil).initializeWithMultiplexer(address(this), mcid, o.initData); + _recordSigil($, pid, o.sigil); + IOutcomeSigil(o.sigil).initializeWithMultiplexer(address(this), ocid, o.initData); } for (uint256 i; i < s.signatureSigils.length; ++i) { SignatureSigilData memory sg = s.signatureSigils[i]; @@ -333,7 +341,8 @@ abstract contract MandateEngine is IMandateEngine { revert UnsupportedSigil(sg.sigil); } $.signatureSigils[pid].add(sg.sigil); - I1271Sigil(sg.sigil).initializeWithMultiplexer(address(this), mcid, sg.initData); + _recordSigil($, pid, sg.sigil); + I1271Sigil(sg.sigil).initializeWithMultiplexer(address(this), scid, sg.initData); } // CEI: enable LAST, after every sigil is registered + initialized. A sigil whose @@ -343,11 +352,33 @@ abstract contract MandateEngine is IMandateEngine { emit MandateBound(pid); } + /// @dev Record `sigil` in the mandate's reverse index ({MandateStorageLib.mandateSigils}) so + /// {EnforcementLib.enforceAction} (which has the executing `pid` in hand) can default-deny any agent call + /// whose `to` is one of this mandate's OWN policy sigils — the confused-deputy guard: a sigil's config is + /// keyed by the account, and the baked-in engine makes the account its own multiplexer, so a + /// mandate-permitted call to a sigil could rewrite the caps that bound the agent. The set is the union of + /// the mandate's action / outcome / signature sigils; it is cleared on revoke/re-bind by + /// {_clearMandateSets}. + /// @param $ The mandate storage pointer. + /// @param pid The mandate registering the sigil. + /// @param sigil The sigil address to record. + function _recordSigil( + MandateStorageLib.MandateStorage storage $, + MandateId pid, + address sigil + ) + private + { + $.mandateSigils[pid].add(sigil); + } + /// @dev Clear EVERY per-mandate enumerable set for `pid`: each action id and its sigil set, the outcome - /// sigils, and the signature (ERC-1271) sigils. Shared by {_revokeMandate} (kill) and - /// {_registerMandate} (re-bind replace), so neither can leave a stale sigil — in particular a stale - /// signature sigil that would keep a 1271 signing capability the new/empty config dropped. Does NOT - /// touch `enabled`, `signerConf`, or `enableNonce`; the callers manage those. + /// sigils, the signature (ERC-1271) sigils, and the {MandateStorageLib.mandateSigils} reverse index. + /// Shared by {_revokeMandate} (kill) and {_registerMandate} (re-bind replace), so neither can leave a + /// stale sigil — in particular a stale signature sigil that would keep a 1271 signing capability the + /// new/empty config dropped, or a stale `mandateSigils` entry that would keep default-denying a target a + /// re-bind no longer treats as a policy sigil. Does NOT touch `enabled`, `signerConf`, or `enableNonce`; + /// the callers manage those. /// @param $ The mandate storage pointer. /// @param pid The mandate whose sets to clear. function _clearMandateSets(MandateStorageLib.MandateStorage storage $, MandateId pid) private { @@ -371,6 +402,11 @@ abstract contract MandateEngine is IMandateEngine { for (uint256 i; i < sgs.length; ++i) { ss.remove(sgs[i]); } + EnumerableSetLib.AddressSet storage ms = $.mandateSigils[pid]; + address[] memory regd = ms.values(); + for (uint256 i; i < regd.length; ++i) { + ms.remove(regd[i]); + } } /// @dev SAFE ERC-165 interface probe used to gate sigil registration per tier. Returns false (never diff --git a/src/lib/EnforcementLib.sol b/src/lib/EnforcementLib.sol index 7c13a51..9670ae7 100644 --- a/src/lib/EnforcementLib.sol +++ b/src/lib/EnforcementLib.sol @@ -11,6 +11,7 @@ import { IOutcomeSigil } from "@interfaces/IOutcomeSigil.sol"; // Libraries import { IdLib } from "@lib/IdLib.sol"; import { MandateStorageLib } from "@lib/MandateStorageLib.sol"; +import { RootStorageLib } from "@lib/RootStorageLib.sol"; // Types import { @@ -65,6 +66,18 @@ library EnforcementLib { address to = to_ == address(0) ? address(this) : to_; if (to == address(this)) return false; // no session self-calls (nested-exec bypass guard) if (to == FALLBACK_TARGET_FLAG) return false; // the fallback sentinel is never a real call target + // The account's auth/policy state is SHARDED across external singletons — its ROOT validators + // ({RootStorageLib}) and its policy sigils — each keyed by the account address and mutated on + // `msg.sender == account`. During a MANDATE execution the account IS `msg.sender` to any non-self target, + // so a call to one of these (e.g. `ECDSAValidator.onInstall` to overwrite the ROOT signer, or a sigil's + // `initializeWithMultiplexer` to raise its own cap) is a self-call in disguise. Default-deny them, the + // sharded-state analogue of the `to == address(this)` guard above (smart-sessions gets this for free — its + // policy module is a SEPARATE address from the account, so the config key never collides; Daimon bakes the + // engine into the account, so it must deny explicitly). Validators are a small account-wide set (full ROOT- + // takeover coverage); sigils are checked against THIS mandate's own set (`pid` is in hand here) — closing + // the headline self-cap-raise. See {MandateStorageLib.mandateSigils} for the cross-mandate residual. + if (RootStorageLib.load().validators.contains(to)) return false; // an installed ROOT validator + if ($.mandateSigils[pid].contains(to)) return false; // one of THIS mandate's own policy sigils ActionId aid = IdLib.toActionId(to, data.length >= 4 ? bytes4(data[0:4]) : bytes4(0)); address[] memory sigils = $.actionSigils[aid][pid].values(); if (sigils.length == 0) { @@ -100,7 +113,7 @@ library EnforcementLib { function runPreChecks(MandateStorageLib.MandateStorage storage $, MandateId pid) internal { address[] memory sigils = $.outcomeSigils[pid].values(); if (sigils.length == 0) return; - ConfigId cid = IdLib.toMandateConfigId(pid); + ConfigId cid = IdLib.toOutcomeConfigId(pid); for (uint256 i; i < sigils.length; ++i) { IOutcomeSigil(sigils[i]).preCheck(cid, address(this)); } @@ -124,7 +137,7 @@ library EnforcementLib { { address[] memory sigils = $.outcomeSigils[pid].values(); if (sigils.length == 0) return; - ConfigId cid = IdLib.toMandateConfigId(pid); + ConfigId cid = IdLib.toOutcomeConfigId(pid); for (uint256 i; i < sigils.length; ++i) { IOutcomeSigil(sigils[i]).postCheck(cid, address(this), mode, executionData); } @@ -150,7 +163,7 @@ library EnforcementLib { returns (bool) { address[] memory sigils = $.signatureSigils[pid].values(); - ConfigId cid = IdLib.toMandateConfigId(pid); + ConfigId cid = IdLib.toSignatureConfigId(pid); for (uint256 i; i < sigils.length; ++i) { if (I1271Sigil(sigils[i]).check1271(cid, address(this), gated) != VALIDATION_SUCCESS) { return false; diff --git a/src/lib/IdLib.sol b/src/lib/IdLib.sol index a47dc90..4b007c6 100644 --- a/src/lib/IdLib.sol +++ b/src/lib/IdLib.sol @@ -30,6 +30,11 @@ library IdLib { } /// @notice Derive the `ConfigId` a sigil stores/reads config under, for a (mandate, action). + /// @dev Cap and usage state is keyed per-mandate (`pid`): two mandates sharing the same session key + /// (same validator + initData but different `salt` → different `MandateId`) carry INDEPENDENT cap + /// state — their caps do NOT aggregate. A mandate builder issuing multiple mandates to one session + /// key must account for this: each mandate's budget is isolated, and the session key can spend up + /// to each mandate's cap independently. /// @param pid The mandate id. /// @param aid The action id. /// @return The sigil config id. @@ -44,8 +49,20 @@ library IdLib { /// never collide with a per-action config id. /// @param pid The mandate id. /// @return The outcome-sigil config id. - function toMandateConfigId(MandateId pid) internal pure returns (ConfigId) { + function toOutcomeConfigId(MandateId pid) internal pure returns (ConfigId) { bytes32 id = keccak256(abi.encodePacked("daimon.outcome", MandateId.unwrap(pid))); return ConfigId.wrap(id); } + + /// @notice Derive the `ConfigId` a per-mandate SIGNATURE (ERC-1271) sigil stores/reads config under, for a + /// mandate. Domain-separated from {toOutcomeConfigId} (outcome) and {toConfigId} (per-action) by a + /// distinct constant tag, so the signature tier can never collide with the outcome tier — even for a + /// single address that implements both {IOutcomeSigil} and {I1271Sigil} and is placed in both slots + /// of one mandate. + /// @param pid The mandate id. + /// @return The signature-sigil config id. + function toSignatureConfigId(MandateId pid) internal pure returns (ConfigId) { + bytes32 id = keccak256(abi.encodePacked("daimon.signature", MandateId.unwrap(pid))); + return ConfigId.wrap(id); + } } diff --git a/src/lib/MandateStorageLib.sol b/src/lib/MandateStorageLib.sol index eb2dbcc..00132ae 100644 --- a/src/lib/MandateStorageLib.sol +++ b/src/lib/MandateStorageLib.sol @@ -49,10 +49,27 @@ library MandateStorageLib { /// @param signatureSigils The set of per-mandate ERC-1271 (attestation) {I1271Sigil}s gating the mandate's /// 1271 signing path — what typed data / content the agent may sign, and for which requesting /// dApp. Empty => the mandate cannot 1271-sign (default-deny). Tracked per mandate so revoke can - /// fully clear them, and their config is keyed by the mandate alone ({IdLib.toMandateConfigId}), - /// like outcome sigils. APPENDED at the end of the struct (after `execNonceUsed`) per the ERC-7201 - /// append-only rule — a new category must never be inserted mid-struct (that would shift every - /// following field's slot and corrupt an in-place upgrade). + /// fully clear them, and their config is keyed by the mandate alone ({IdLib.toSignatureConfigId}) — + /// domain-separated from the outcome tier's {IdLib.toOutcomeConfigId} so the two never collide. + /// APPENDED after `execNonceUsed` per the ERC-7201 append-only rule — a new category must never be + /// inserted mid-struct (that would shift every following field's slot and corrupt an in-place upgrade). + /// @param mandateSigils Per-mandate reverse index: the UNION of every sigil address (action / outcome / + /// signature) the mandate registered, keyed by {MandateId}. A sigil holds this account's policy config + /// keyed by `(configId, msg.sender == account)` — and because the engine is BAKED INTO the account, the + /// configuring "multiplexer" IS the account, so a mandate-permitted call to a sigil (also + /// `msg.sender == account`) hits the IDENTICAL config key and can rewrite the very caps/policy that + /// bound the agent (raise its own SpendSigil cap, etc.). This is the confused-deputy surface + /// smart-sessions avoids for free by keeping its module a SEPARATE address from the account + /// (multiplexer != account); Daimon cannot, so the MANDATE path ({EnforcementLib.enforceAction}) — which + /// already has the executing `pid` in hand — default-denies any call whose `to` is one of the mandate's + /// OWN sigils, the sigil-side analogue of the `to == address(this)` self-call guard (config state is + /// sharded across external sigil singletons, so denying `address(this)` alone is insufficient). RESIDUAL: + /// this blocks a mandate from rewriting ITS OWN caps (the headline confused-deputy path); it does NOT + /// block a broad mandate from rewriting a DIFFERENT mandate's sigil config — only reachable when one + /// compromised key holds both mandates (see the SpendSigil composition residual). ROOT takeover/brick is + /// covered separately and fully by the {RootStorageLib} validator-set check. Cleared on revoke/re-bind + /// alongside the other per-mandate sets. APPENDED at the end of the struct per the ERC-7201 append-only + /// rule. /// @dev The Mandate's `validUntil` is the BIND-AUTHORIZATION DEADLINE — committed to the MANDATE_BIND digest /// (so the ROOT signer authenticates it) and enforced ONCE at bind time by {MandateEngine}; it is NOT /// persisted in this struct and NOT consulted at runtime. RUNTIME time bounds live in a {TimeFrameSigil} @@ -67,6 +84,7 @@ library MandateStorageLib { mapping(MandateId => EnumerableSetLib.AddressSet) outcomeSigils; mapping(uint256 => bool) execNonceUsed; mapping(MandateId => EnumerableSetLib.AddressSet) signatureSigils; + mapping(MandateId => EnumerableSetLib.AddressSet) mandateSigils; } /// @dev ERC-7201 namespaced storage slot for `daimon.storage.mandate.v1`. Derived as diff --git a/src/sigils/AttestationSigil/AttestationSigil.sol b/src/sigils/AttestationSigil/AttestationSigil.sol index 337a201..0287158 100644 --- a/src/sigils/AttestationSigil/AttestationSigil.sol +++ b/src/sigils/AttestationSigil/AttestationSigil.sol @@ -52,7 +52,10 @@ import { /// /// 2. STATELESS + VIEW. The 1271 path runs under solady's STATICCALL, so {check1271} is `view` and /// writes no state — the gate is purely an allowlist membership test (no usage accrual; the digest -/// + session-key binding live in the engine). +/// + session-key binding live in the engine). Like {Eip3009Sigil}, there is intentionally NO +/// cumulative or frequency cap on this signing path: a compromised session key can produce unlimited +/// fresh valid attestations up to whatever the consuming protocol or funding level allows. Mitigate +/// by minimising per-mandate funding and using short bind windows — revoke promptly on compromise. /// /// Configured per account by the {MandateEngine} at bind time via {initializeWithMultiplexer}; keyed by /// `(configId, msg.sender, account)`. The engine is baked into the account, so `msg.sender == account` diff --git a/src/sigils/NativeValueLimitSigil/NativeValueLimitSigil.sol b/src/sigils/NativeValueLimitSigil/NativeValueLimitSigil.sol index d5f8ca4..25b3692 100644 --- a/src/sigils/NativeValueLimitSigil/NativeValueLimitSigil.sol +++ b/src/sigils/NativeValueLimitSigil/NativeValueLimitSigil.sol @@ -71,7 +71,11 @@ contract NativeValueLimitSigil is IActionSigil { /*·:⛧:·──────── CHECK ────────:⛧:·*/ /// @inheritdoc IActionSigil - /// @dev Permits iff `value <= limit`. Reads no `data` (no argument logic) — native value is the only constraint. + /// @dev Permits iff `value <= limit`. Reads no `data` (no argument logic) — native value is the only + /// constraint. This bound is PER-CALL, not per-execution: in a K-call batch each individual call + /// may carry up to `limit` wei, so the total native outflow in one execution can reach K·limit. + /// For a cumulative per-execution or per-period native ceiling, compose with a NATIVE-budget + /// {SpendSigil} (set `token` to the NATIVE sentinel `0xEeee…EEeE`). function checkAction( ConfigId id, address account, diff --git a/src/sigils/OmniSigil/lib/OmniSigilTreeLib.sol b/src/sigils/OmniSigil/lib/OmniSigilTreeLib.sol index 251c455..102a228 100644 --- a/src/sigils/OmniSigil/lib/OmniSigilTreeLib.sol +++ b/src/sigils/OmniSigil/lib/OmniSigilTreeLib.sol @@ -25,6 +25,9 @@ library OmniSigilTreeLib { error TooManyNodes(); /// @notice Thrown when a node references a child index out of bounds. error NodeChildIndexOutOfBounds(); + /// @notice Thrown when a node references a child index that is not strictly less than its own index — a + /// forward/self reference that would let the node graph contain a cycle. + error NodeChildIndexNotDescending(); /// @notice Thrown when a leaf node references a rule index out of bounds. error RuleIndexOutOfBounds(); @@ -94,7 +97,16 @@ library OmniSigilTreeLib { return true; } - /// @notice Validate that the expression tree is well-formed (bounds + child/rule indices). + /// @notice Validate that the expression tree is well-formed (bounds + child/rule indices) AND acyclic. + /// @dev Acyclicity: every child index must be STRICTLY LESS than its parent node's own index. This single + /// topological constraint guarantees the node graph is a DAG (it admits no self- or back-edge), which + /// bounds {evaluateNode}'s recursion depth to `nodeCount` — closing the unbounded-recursion (OOG) brick + /// where a node whose child points to itself or an ancestor (e.g. an AND at index 0 with `leftChild == 0`) + /// would pass the bounds check yet recurse forever. The constraint is free for every real tree: the SDK + /// builders emit nodes children-first (a leaf/subtree always precedes the operator that consumes it), so + /// a child index is already < its parent's. The out-of-bounds checks run FIRST, so a child index + /// `>= nodeCount` still reverts {NodeChildIndexOutOfBounds}; an in-bounds non-descending child reverts + /// {NodeChildIndexNotDescending}. /// @param rules The rule set + tree to validate. function validateExpressionTree(ParamRules memory rules) internal pure { uint256 nodeCount = rules.packedNodes.length; @@ -112,12 +124,14 @@ library OmniSigilTreeLib { if (nodeType == NODE_TYPE_RULE) { require(node.getRuleIndex() < ruleCount, RuleIndexOutOfBounds()); } else if (nodeType == NODE_TYPE_NOT) { - require(node.getLeftChildIndex() < nodeCount, NodeChildIndexOutOfBounds()); + uint8 left = node.getLeftChildIndex(); + require(left < nodeCount, NodeChildIndexOutOfBounds()); + require(left < i, NodeChildIndexNotDescending()); } else { - require( - node.getLeftChildIndex() < nodeCount && node.getRightChildIndex() < nodeCount, - NodeChildIndexOutOfBounds() - ); + uint8 left = node.getLeftChildIndex(); + uint8 right = node.getRightChildIndex(); + require(left < nodeCount && right < nodeCount, NodeChildIndexOutOfBounds()); + require(left < i && right < i, NodeChildIndexNotDescending()); } } } @@ -173,6 +187,11 @@ library OmniSigilTreeLib { /*·:⛧:·──────── FILL ────────:⛧:·*/ /// @notice Copy a memory config into storage (clean slate). + /// @dev `delete $config.paramRules.rules` followed by re-pushing resets each rule's cumulative + /// `usage.used` counter to 0. This DIFFERS from {SpendSigil} and {RateLimitSigil}, which + /// deliberately preserve their rolling state across re-init so a re-bind cannot clear an exhausted + /// budget. OmniSigil's cumulative argument-level limits ARE reset on every ROOT re-bind — intentional + /// clean-slate semantics, restricted to the ROOT tier. /// @param $config The destination storage config. /// @param config The source memory config. function fill(ActionConfig storage $config, ActionConfig memory config) internal { diff --git a/src/sigils/RateLimitSigil/lib/RateLimitConfigLib.sol b/src/sigils/RateLimitSigil/lib/RateLimitConfigLib.sol index 6919ed8..76b82f0 100644 --- a/src/sigils/RateLimitSigil/lib/RateLimitConfigLib.sol +++ b/src/sigils/RateLimitSigil/lib/RateLimitConfigLib.sol @@ -24,6 +24,10 @@ struct RateLimitConfig { /// it). The window is `[windowStart, windowStart + windowSeconds)`; a later action rolls it forward. /// @param count Number of permitted actions charged within the current window. /// @param lastActionAt Unix-seconds timestamp of the last permitted action (drives the cooldown check). +/// @dev All three timestamp fields are `uint32`: `block.timestamp` is cast to `uint32` on every write and +/// compared as `uint32`, so it wraps to zero after ~year 2106. Past that horizon the window/cooldown +/// comparisons mis-evaluate (regardless of any mandate fields); the sigil is not designed to operate +/// beyond it. struct RateLimitState { uint32 windowStart; uint32 count; diff --git a/src/sigils/SpendSigil/SpendSigil.sol b/src/sigils/SpendSigil/SpendSigil.sol index d46d04e..61ccb4b 100644 --- a/src/sigils/SpendSigil/SpendSigil.sol +++ b/src/sigils/SpendSigil/SpendSigil.sol @@ -28,10 +28,10 @@ import { IOutcomeSigil } from "@interfaces/IOutcomeSigil.sol"; /// outflow = max(calldata-sum, balanceBefore − balanceAfter) /// spent += outflow ≤ cap · no dangling allowance // forgefmt: disable-end -/// @title SpendSigil — a stateful, rolling-window spend cap (pure outcome guard) +/// @title SpendSigil — a stateful, tumbling-window spend cap (pure outcome guard) /// @author highskore.eth /// @notice A per-execution {IOutcomeSigil} that meters one budgeted asset's net outflow from the account against -/// a cap that resets on a rolling window. The budgeted asset is either an ERC-20 OR — when `token` is the +/// a cap that resets on a calendar/tumbling window. The budgeted asset is either an ERC-20 OR — when `token` is the /// {NATIVE} sentinel (`0xEeee…EEeE`) — native ETH, bounding a self-relaying agent's native spend per /// period. For an ERC-20 it closes the approval-bypass gap (ERC-1608 §Security): {postCheck} itemizes /// EVERY executed call (the ERC-7579 set it is handed), summing each transfer/approve outflow of the @@ -46,7 +46,7 @@ import { IOutcomeSigil } from "@interfaces/IOutcomeSigil.sol"; /// snapshots `balanceBefore` in EIP-1153 TRANSIENT storage (cancun), keyed by `(account, msg.sender, /// token)` — the budgeted TOKEN, not the ConfigId. Transient storage auto-clears at end-of-tx, and /// {preCheck} re-snapshots per execution so two executions bracketed in one transaction never leak. Only -/// the rolling `SpendState` is persistent. +/// the tumbling `SpendState` is persistent. /// /// Like every sigil it keys config by `(configId, msg.sender, account)`; the engine is baked into the /// account, so `msg.sender == account` at runtime. This is a PURE outcome sigil: it has no ERC-1271 tier — @@ -66,8 +66,12 @@ import { IOutcomeSigil } from "@interfaces/IOutcomeSigil.sol"; /// parse cannot see (and it matches the prior per-call model — a non-token target contributed zero to the /// sum there too). It is bounded by the controls OUTSIDE the meter: a bounded agent cannot establish the /// pull authority such a sink needs — an in-batch `approve`/`increaseAllowance` IS parsed and charged here, -/// blanket grants (permit / setApprovalForAll / authorizeOperator) revert {BlanketGrantBlocked}, and any -/// approve-spender left dangling reverts {DanglingAllowance} — so a standing allowance to an unparsed sink +/// blanket grants (permit / setApprovalForAll / authorizeOperator) revert {BlanketGrantBlocked} (note: +/// ERC-777 `send(address,uint256,bytes)` / `operatorSend` on the BUDGETED TOKEN ITSELF are a concrete +/// example of an unparsed direct-outflow selector — they move tokens without matching the parsed +/// transfer/approve selectors and are metered only by the maskable balance delta the residual describes, +/// distinct from the blocked `authorizeOperator` operator-grant), and any approve-spender left dangling +/// reverts {DanglingAllowance} — so a standing allowance to an unparsed sink /// can only come from the ROOT (owner) tier, which is unconstrained by design (ROOT bypasses the cap /// outright). Within the bounded-agent threat model the residual is therefore unreachable; the per-action /// allowlist further constrains which targets an agent may call at all. @@ -264,10 +268,10 @@ contract SpendSigil is IOutcomeSigil { /*·:⛧:·──────── INTERNAL: METER + ACCRUE ────────:⛧:·*/ - /// @dev Charge `max(byCalldata, real balance delta)` to the rolling spend and enforce the cap. The balance - /// delta catches outflows the calldata parse undercounts (an unparsed target, a pull via a granted - /// allowance, a flash trick); the global calldata sum catches outflows a same-execution inflow would - /// mask in the delta. Rolls the period (resetting `spent` across a window boundary) before accruing. + /// @dev Charge `max(byCalldata, real balance delta)` to the tumbling-window spend and enforce the cap. The + /// balance delta catches outflows the calldata parse undercounts (an unparsed target, a pull via a + /// granted allowance, a flash trick); the global calldata sum catches outflows a same-execution inflow + /// would mask in the delta. Rolls the period (resetting `spent` across a calendar boundary) before accruing. function _accrue(ConfigId id, address account, address token, uint256 byCalldata) private { SpendConfig storage cfg = id.getConfig(msg.sender, account); uint256 delta = @@ -306,12 +310,17 @@ contract SpendSigil is IOutcomeSigil { /*·:⛧:·──────── PERIOD ────────:⛧:·*/ - /// @notice Round `ts` down to the start of its `period` window — the boundary the rolling cap resets - /// on. A charge whose `lastUpdated` predates this boundary belongs to a closed window, so the - /// running `spent` is reset to zero before accruing. - /// @param period The rolling window. + /// @notice Round `ts` down to the start of its `period` window — the boundary the cap resets on. A + /// charge whose `lastUpdated` predates this boundary belongs to a closed window, so the running + /// `spent` is reset to zero before accruing. + /// @dev FIXED / TUMBLING window: boundaries are calendar-aligned (Minute, Hour, Day, Week, Month, Year), + /// NOT anchored to the first spend. Because both sides of a boundary belong to separate windows, up + /// to 2× the cap can be spent across a single period-length span that straddles a boundary (the + /// tail of one period plus the head of the next). This mirrors {RateLimitSigil}'s window model; + /// a true sliding window would require unbounded per-charge timestamp storage. + /// @param period The tumbling window. /// @param ts The timestamp to round down. - /// @return The unix-seconds start of the window containing `ts` (0 for `Forever`, so nothing resets). + /// @return The unix-seconds start of the calendar window containing `ts` (0 for `Forever`, so nothing resets). function startOfPeriod(Period period, uint256 ts) public pure returns (uint256) { if (period == Period.Minute) return ts - (ts % MINUTE); if (period == Period.Hour) return ts - (ts % HOUR); diff --git a/src/sigils/TimeFrameSigil/TimeFrameSigil.sol b/src/sigils/TimeFrameSigil/TimeFrameSigil.sol index 7073ed9..5040e58 100644 --- a/src/sigils/TimeFrameSigil/TimeFrameSigil.sol +++ b/src/sigils/TimeFrameSigil/TimeFrameSigil.sol @@ -134,6 +134,12 @@ contract TimeFrameSigil is IActionSigil, I1271Sigil { /// @inheritdoc I1271Sigil /// @dev Mirrors {checkAction}: enforces the same `[validAfter, validUntil]` window on the ERC-1271 path, /// reading no `content`. A signed message authorized under this policy is only valid inside the window. + /// + /// TIME-ONLY gate: because `content` is ignored, placing this sigil ALONE in a mandate's signature + /// slot would authorize signing ANY digest for ANY requesting dApp within the window — it is a + /// REFINEMENT of the signing scope, not a standalone authorization. It must always be composed with + /// a content/sender-binding signature sigil ({AttestationSigil} or {Eip3009Sigil}) that constrains + /// WHAT can be signed; TimeFrameSigil then constrains WHEN. function check1271( ConfigId id, address account, @@ -149,6 +155,14 @@ contract TimeFrameSigil is IActionSigil, I1271Sigil { /// @dev The shared window check used by BOTH {checkAction} and {check1271}. /// Returns {VALIDATION_SUCCESS} iff `block.timestamp >= validAfter` AND /// (`validUntil == 0` OR `block.timestamp <= validUntil`); otherwise {VALIDATION_FAILED}. + /// + /// FAIL-OPEN DEFAULT: this is the only sigil with no initialized-guard. An unconfigured + /// `(validAfter=0, validUntil=0)` config returns VALIDATION_SUCCESS — fail-open. This is safe only + /// because the engine guarantees {initializeWithMultiplexer} runs for every bound action/signature + /// sigil under the matching configId before any check; do NOT invoke `_check` on a possibly- + /// uninitialized triple from any other call path. A `(0,0)` window is also intentionally bindable + /// as "open-ended" (no lower bound, no upper bound), so the fail-open default and a deliberate + /// open-ended binding are indistinguishable by design. /// @param cfg The stored time-window config for (id, multiplexer, account). /// @return A validation code: `VALIDATION_SUCCESS` (in window) or `VALIDATION_FAILED` (outside). function _check(TimeFrameConfig storage cfg) private view returns (uint256) { diff --git a/test/integration/Daimon/spendSigil/spendSigil.t.sol b/test/integration/Daimon/spendSigil/spendSigil.t.sol index 5527988..8329664 100644 --- a/test/integration/Daimon/spendSigil/spendSigil.t.sol +++ b/test/integration/Daimon/spendSigil/spendSigil.t.sol @@ -362,7 +362,7 @@ contract Daimon_spendSigil_Integration_Test is Daimon_Integration_Test { /// @dev The outcome-sigil ConfigId for this mandate (per-execution, keyed by the mandate alone). function _cid() internal view returns (ConfigId) { - return IdLib.toMandateConfigId(_spendSigilMandateId()); + return IdLib.toOutcomeConfigId(_spendSigilMandateId()); } /// @dev Bind the spend-sigil mandate inline at the given exec `nonce`, via a no-op self-transfer of 0. diff --git a/test/integration/Daimon/swapWithApprove/swapWithApprove.t.sol b/test/integration/Daimon/swapWithApprove/swapWithApprove.t.sol index 67a6038..ebd8242 100644 --- a/test/integration/Daimon/swapWithApprove/swapWithApprove.t.sol +++ b/test/integration/Daimon/swapWithApprove/swapWithApprove.t.sol @@ -302,7 +302,7 @@ contract Daimon_swapWithApprove_Integration_Test is Daimon_Integration_Test { /// @dev The outcome-sigil ConfigId for this mandate (per-execution, keyed by the mandate alone). function _cid() internal view returns (ConfigId) { - return IdLib.toMandateConfigId(_swapMandateId()); + return IdLib.toOutcomeConfigId(_swapMandateId()); } /// @dev A BIND+USE signature: the inline genesis-style mandate BIND (ROOT-signed) packed with the agent's