security: registerDINaggregator has no onlyCurrentGI — validators can pre-register for (and fill) a future GI's aggregator list #206

Summary

Found while reviewing PR No. 204 (verification comment, surfaced item B). This is older than PR No. 204 and out of its scope. Line numbers are for develop @ e373c8d.

DINTaskCoordinator.registerDINaggregator(_GI) doesn't check that _GI is the current GI. While any GI's aggregator registration window is open, a validator can register for a future GI, which fills that GI's aggregator list before its own window opens.

Problem

foundry/src/DINTaskCoordinator.sol:397:

function registerDINaggregator(uint _GI) public {
    if (GIstate != GIstates.DINaggregatorsRegistrationStarted) revert ...;
    ...
    if (isDINAggregator[_GI][msg.sender]) revert TC_AggregatorAlreadyRegistered();
    if (dinAggregators[_GI].length >= MAX_REGISTERED_AGGREGATORS) revert TC_RegistrationCapReached();
    ...
    dinAggregators[_GI].push(msg.sender);
    isDINAggregator[_GI][msg.sender] = true;
    dinvalidatorStakeContract.incrementActiveRegistration(msg.sender);

The state check is on the current GI, but every read and write uses the caller's _GI. The auditor side does check: DINTaskAuditor.registerDINAuditor(uint _GI) public onlyCurrentGI(_GI) (DINTaskAuditor.sol:660), and the coordinator has the same modifier (L246). dincli always passes curr_GI (dincli/cli/aggregator.py:139), so honest users are unaffected.

Failure scenario

Proven with a throwaway forge test (run, deleted, not committed): during GI 1's registration window, registerDINaggregator(2) succeeds. isDINAggregator(2, v) becomes true, isDINAggregator(1, v) stays false, and activeRegistrationCount(v) == 1.

Consequences:

1. Capturing a future GI's aggregation. During GI N's window, an attacker registers 300 addresses (MAX_REGISTERED_AGGREGATORS) for GI N+1. When GI N+1's window opens, every honest validator hits TC_RegistrationCapReached, and autoCreateTier1AndTier2 builds all T1/T2 batches from the attacker's addresses (those still Active). They pick every batch's CID. The only defense is an owner-adjudicated S4 dispute. The cost is 300 × MIN_STAKE (10 DIN by default, about 0.003 ETH at the default faucet rate), minus any per-model floor or concurrency cap if configured. In normal registration, honest validators at least compete inside the window; pre-registration removes that.
2. Bypassing the owner's timing. Registrations land for a GI whose window the owner never opened.
3. Leaked registration slots. Registering for an already-ended GI whose slots were released increments activeRegistrationCount. releaseGIRegistrationSlots (L1261) won't run again for that GI, so the slot is never freed. This only affects the caller.

Fix

Add onlyCurrentGI(_GI) to registerDINaggregator, matching registerDINAuditor, plus a regression test: registerDINaggregator(GI + 1) and registerDINaggregator(GI - 1) revert with TC_WrongGI.

Size note: DINTaskCoordinator is already 9 B over EIP-170 on develop (No. 201 Part A), so this change must land together with, or after, No. 201's size reduction. Check forge build --sizes in the PR.

Related

No. 201 (contract size), No. 180 (dual-role registration), PR No. 204.