Skip to content

security: dual-role registration allows one operator to control both the auditor filter and T1 aggregator in the same GI #180

New issue
New issue
Closed
Closed
security: dual-role registration allows one operator to control both the auditor filter and T1 aggregator in the same GI#180
Labels
enhancementNew feature or request

Description

@robertocarlous
robertocarlous
opened on Sep 28, 2026

Summary

Nothing in DINTaskCoordinator.registerDINaggregator or DINTaskAuditor.registerDINAuditor prevents a single validator from registering in both roles for the same GI. Only the concurrent-registration cap (per-stake limit on total active registrations across all task contracts) applies.

A dual-role operator controls both:

  • the approved-model filter (via the auditor batch they sit in), and
  • the T1 aggregation step that consumes that filter output.

Combined with the grindable T1 batch shuffle (#156), the model owner can route the poisoned model directly into the colluding T1 batch, making the attack require zero off-chain coordination beyond the operator's own keys.

Impact

The attack surface is: colluding auditors approve a poisoned client model (S3 KNOWN GAP), colluding T1 aggregators fold it in unclipped (off-chain), and the model owner routes it into the right T1 batch. With dual-role registration all three roles can collapse to one operator.

Decision needed

Is dual-role registration intentional? If yes, document it as a trust assumption. If not, add a registration guard: either a cross-contract isDINAggregator check at auditor registration time, or a platform-level role-separation flag.

Tracking

Surfaced in the adversarial threat-model doc (task_240926_17, PR 1).

Activity

  1. robertocarlous
    added
    enhancementNew feature or request
    on Sep 28, 2026

  2. umeradl commented on Sep 28, 2026

    @umeradl
    umeradl
    on Sep 28, 2026
    Member

    Approval review: Changes requested

    Verified against origin/develop @ 37b2daf.

    Claims

    Claim Result Evidence
    Nothing in registerDINaggregator / registerDINAuditor blocks one address from holding both roles in a GI ✅ DINTaskCoordinator.sol:367-400 and DINTaskAuditor.sol:660-693 only check the per-contract duplicate flag, the active-validator status, the registration cap, the per-model stake floor and the concurrent cap
    "Only the concurrent-registration cap applies" ⚠️ The per-model stake floor (getModelStakeMin) also applies. The concurrent cap is enforced only when maxConcurrentRegistrationsPerStakeUnit > 0, and it is unset (0) by default, so out of the box nothing limits registrations. Both setters' natspec still say "(not yet enforced)" (DinValidatorStake.sol:451, :462), which is stale
    The T1 batch shuffle is grindable (No. 156) ✅ autoCreateTier1AndTier2 is onlyOwner. Aggregators are shuffled by blockhash(block.number - 1) (DINTaskCoordinator.sol:541) and approved models by block.timestamp, msg.sender (:617-625), so the owner picks the call block
    "All three roles can collapse to one operator" ❌ as a single address T1 batches have T1_AGGREGATORS_PER_BATCH = 3 with a majority CID (:40, :773). Auditor eligibility needs minEligibilityQuorum yes-votes (demo: 3 per batch, quorum 2; DINTaskAuditor.sol:391, :1052). The duplicate checks stop one address taking 2 seats in either role, so the attack needs at least 2 colluding addresses per role whether or not dual-role is allowed

    Merit

    The attack in the issue is real, but it's a Sybil-plus-owner-grinding attack, not a dual-role one. A same-address guard (option B) is bypassed by registering the auditor seat from a second address. The only added cost is one more minStake / model-floor stake. It also needs a cross-contract call on every auditor registration, and it would stop honest validators from filling both roles on models with thin validator pools. Neither isDINAggregator[_GI] nor isRegisteredAuditor[_GI] identifies a person, so a role-separation flag has the same weakness.

    The parts that make the attack cheap are fixed elsewhere:

    • the owner-grindable T1 routing (No. 156);
    • the concurrent cap and model stake floor, which set the Sybil cost per extra address;
    • the off-chain clipping gap in T1, which is model-service logic.

    Amendments

    1. Take option A: document dual-role registration as an intended trust assumption. In PR No. 182, Row 6's "Trust assumption" should say this, with the cost argument above. "Expected test outcome" should stop calling the registration itself the gap, and the proposed-fix table row pointing here should be removed.
    2. In PR No. 182 Row 6, fix the stale reference DINTaskAuditor.sol:188 → :660, and note that the aggregator shuffle seed is blockhash(block.number - 1); only the model-index shuffle uses block.timestamp + msg.sender.
    3. Keep the routing half of this attack tracked under No. 156. Once PR No. 182 reflects the decision, this issue can be closed as "working as intended, documented".
    4. Optional, separate one-liner: drop the stale "(not yet enforced)" from the natspec at DinValidatorStake.sol:451 and :462.

    Conflicts

    • PR No. 182 (Developer/design/adversarial-threat-model.md): Row 6 and its open question 2 depend on this decision; amendments 1–2 apply there.
    • PR No. 31 and No. 32 touch foundry/src/DINTaskAuditor.sol, which matters only if a guard were added. None if option A is taken.
  3. umeradl
    added a commit that references this issue on Sep 28, 2026

    docs(issue-154): adversarial threat model — scenario matrix, trust as…

    e9a0fe9

  4. umeradl commented on Sep 30, 2026

    @umeradl
    umeradl
    on Sep 30, 2026
    Member

    Status update: not closable yet

    Checked on origin/develop @ 6ccc28c.

    The approval review recommended option A (document dual-role as intended) and closing this issue afterwards. The PR No. 182 review then decided the opposite. Developer/design/adversarial-threat-model.md open question 2 now reads "Row 6 — Dual-role registration: KNOWN GAP. Not accepted as intended; No. 180 stays open", and it leaves the choice between a guard and documenting the stake cost to this issue. That supersedes amendments 1 and 3.

    Item Status Evidence
    Decision: add a guard, or document stake cost as the defence ❌ Open. Row 6 is KNOWN GAP and the gaps-before-audit table points here
    Amendment 2: Row 6 line ref :188 → :660 ✅ Row 6 cites DINTaskAuditor.sol:660
    Amendment 2: note on the shuffle seed ✅ (superseded) The routing half closed with No. 156 H-2 (PR No. 191): batch creation now reads a locked aggSeed / auditSeed. Row 6 documents this, with the BL-26 re-roll left as a residual
    Amendment 4: stale "(not yet enforced)" NatSpec ❌ Still at DinValidatorStake.sol:451 and :462

    Remaining

    1. Decide between a per-address cross-role guard and documenting the stake cost as the defence. If the stake cost is the defence, pick a non-zero default for maxConcurrentRegistrationsPerStakeUnit. Then update Row 6 and flip its test outcome.
    2. The one-line NatSpec fix from amendment 4.
  5. umermjd11
    added 2 commits that reference this issue on Oct 5, 2026

    docs(tasks-plan): narrow 051026-1 to contract follow-ups (InfiniteZer…

    a9b5f27

    docs(tasks-plan): 051026-1 — apply PR No. 219 review amendments 1-5 a…

    b797f36
  6. umeradl
    added 2 commits that reference this issue on Oct 6, 2026

    docs(tasks-plan): umermjd11 task plan 051026-1 — DINTaskAuditor size (#…

    aeeb4d7

    docs(tasks): add task_061026_20 and task_061026_21 (forwarded from ta…

    d95d946
  7. robertocarlous
    added a commit that references this issue on Oct 7, 2026

    test(adversarial): address PR InfiniteZeroFoundation#218 review fixes…

    1b34776
  8. umeradl
    added 2 commits that reference this issue on Oct 7, 2026

    fix(task-auditor): one address can't hold both roles in a GI (#180)

    c4c2b1c

    merge: PR No. 235 (umermjd11/feat/task-061026-20-auditor-size-dual-ro…

    e66d7bc

  9. umeradl commented on Oct 7, 2026

    @umeradl
    umeradl
    on Oct 7, 2026
    Member

    Done by PR No. 235, merged into develop as e66d7bc, with review fixes in 5022604 (outcome comment). I'm closing this by hand, because a merge into develop doesn't trigger closing keywords (main is the default branch).

    The status update left the decision open. PR No. 219 (Decision 2) settled it: a per-address guard, implemented as task_061026_20 Part B.

    Checked on origin/develop @ e66d7bc:

    • Guard: registerDINAuditor reverts with TA_DualRoleNotAllowed when the caller is already an aggregator for the same GI (DINTaskAuditor.sol:681-682). It reads isDINAggregator, which is now on IDINTaskCoordinator (DINShared.sol:119); the error is at DINShared.sol:376. The guard is on the auditor side, because aggregator registration (states 6–7) always comes before auditor registration (8–9).
    • Tests (StakingEnforcement.t.sol):
      • test_registerDINAuditor_aggregatorSameGI_reverts (:493). In review this test failed with the guard removed.
      • test_registerDINAuditor_aggregatorOfEarlierGI_registers (:512): the guard is per GI.
      • test_registerDINAuditor_secondAddressWithOwnStake_registers (:544): documents the Sybil limit.
    • Amendment 2 and the line refs: threat model Row 6 now cites DINTaskAuditor.sol:674 and DINTaskCoordinator.sol:401.
    • Amendment 4 (NatSpec): "(not yet enforced)" is gone from DinValidatorStake.sol. The setModelStakeBounds / setMaxConcurrentRegistrationsPerStakeUnit NatSpec now names where the task contracts enforce each value. activeRegistrationCount names releaseGIRegistrationSlots as the decrement (:121). getModelStakeMin names the onlyOwner (DIN-Representative) setter.
    • Threat model: Row 6's outcome is now DEFENDED (single address). The Gaps table and open question 2 are updated, with the Sybil caveat: a second address with its own stake still gets both roles, at the cost of a second stake.
    • Docs: DINTaskAuditor.md (registration checks and change log), DINShared.md (interface and error rows) and DinValidatorStake.md caveat No. 4.
    • Verification: the via_ir build and size gate pass. DINTaskAuditor is 21,290 B, a 3,286 B margin. forge test gives 501 passed, 0 failed. The develop push CI run on the merge commit passed.

    Out of scope:

    • Sybil variant: accepted residual, documented in Row 6 and bounded by stake cost. Not tracked as an issue.
    • PR No. 218's test_knownGap_dualRoleRegistration: PR No. 218 is still open. It flips that test to test_defended_… when it rebases on this change, as Row 6 and task_061026_20 note.
    • The routing half (owner block-selection): closed under No. 156 (PR No. 191), with the residuals in BL-26.
  10. umeradl
    closed this as completedon Oct 7, 2026
  11. robertocarlous
    added a commit that references this issue on Oct 11, 2026

    fix(task-auditor): one address can't hold both roles in a GI (Infinit…

    329a585
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions