fix(security): confused-deputy deny + reentrancy guard + tree acyclicity + config-id split (F1/F2/G/I) - #1
Merged
Conversation
…ity + 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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ported from rhinestonewtf/daimon#186. D excluded (invalid finding); deadline/Execute digest untouched.
F1 — confused-deputy default-deny
EnforcementLib.enforceActionnow default-denies a MANDATE call whosetois an installed ROOT validator (RootStorageLib) or one of THIS mandate's own policy sigils — the sharded-state analogue of the existingto == address(this)self-call guard. A new per-mandate reverse indexmandateSigils(MandateStorageLib, appended per ERC-7201 append-only) is populated via_recordSigilfor every action/outcome/signature sigil in_registerMandateand cleared in_clearMandateSets.F2 — reentrancy guard
Daimoninherits soladyReentrancyGuardTransient;executeWithSigis nownonReentrant. A nestedexecuteWithSigwould clobber an outer execution's SpendSigil transient balance snapshot. The existing ERC-1608deadlinesignature is preserved.G — expression-tree acyclicity
OmniSigilTreeLib.validateExpressionTreerequires every child index to be STRICTLY LESS than its parent's own index (newNodeChildIndexNotDescendingerror), after the existing out-of-bounds checks. This guarantees the node graph is a DAG and boundsevaluateNoderecursion tonodeCount, closing an unbounded-recursion (OOG) brick.I — config-id split
IdLib.toMandateConfigId→toOutcomeConfigId(keeps the"daimon.outcome"tag); newtoSignatureConfigId("daimon.signature"tag) domain-separates the ERC-1271 tier from the outcome tier, so a single address serving both tiers can never collide. Callers updated (MandateEnginesignature→signature / outcome→outcome,EnforcementLib.enforce1271→signature, pre/post→outcome, plus two test helpers).Natspec-only clarifications
SpendSigil (rolling→tumbling window + ERC-777 send/operatorSend residual), NativeValueLimitSigil (per-call bound), RateLimitConfigLib (uint32 ~2106 wrap from the block.timestamp cast), TimeFrameSigil (
_checkfail-open default + check1271 time-only/compose), AttestationSigil (no cumulative cap), OmniSigilTreeLib.fill (usage.used reset on ROOT re-bind), IdLib (per-mandate cap non-aggregation).Excluded
D (subMode/mandateId in the exec digest) — reverted upstream as an invalid finding; this repo's
Execute+deadlinedigest design is left untouched.Verification
forge buildclean ·forge test450 passed, 0 failed.🤖 Generated with Claude Code