security: anyone can drain a GI reward pool through test-data disputes (unauthenticated resolveTestDataDispute, zero default bond) #205

Summary

Found while reviewing PR No. 204 (verification comment, surfaced item A). This is older than PR No. 204 and out of its scope. PR No. 204 records it as a caveat in DINTaskAuditor.md §13 No. 1, but nothing tracks a fix. Line numbers are for develop @ e373c8d.

Any address can take 25% of a GI's reward pool per batch through a test-data dispute. It needs no DIN, no stake, and no role in the batch. The attack can be repeated after every reassignment, and each round also blocks the batch.

Problem

foundry/src/DINTaskAuditor.sol:

• openTestDataDispute(gi, batchId) (L1461) has no caller check. It only needs a stored commitment, no active dispute, and no pending reassignment. It pulls disputeBondAmount, which is 0 at deploy (L283).
• resolveTestDataDispute(gi, batchId, K, plaintextHash) (L1495) is also callable by anyone. Its NatSpec says so: "Anyone may call this — the disputer is the beneficiary if upheld". It recomputes keccak256(gi, batchId, keccak256(K), plaintextHash) and compares it with the owner's commitment:
  • match (L1510) → dispute false, bond forfeited;
  • mismatch → dispute upheld: bond returned, disputePenaltyBps (2500 = 25%, L285) of giRewardPool[gi] burned or forwarded via _burnAndForward (L1527), and the batch set to pendingReassignment.

A mismatch can't tell "the owner committed bad test data" apart from "the caller passed a made-up K". K and plaintextHash are both caller-supplied and neither is checked independently, so the caller controls which branch runs.

Failure scenario

Proven with a throwaway forge test that extends EncryptedTestDataTest. It was run and deleted, not committed.

1. The model owner assigns batch 0's test data with a valid commitment (assignAuditTestDataset).
2. outsider has no DIN, isn't a batch auditor, and the bond is the default 0. They call openTestDataDispute(1, 0), then resolveTestDataDispute(1, 0, junkK, 0x0).
3. The dispute is upheld: giRewardPool(1) drops 25%, and batch 0 is pendingReassignment until the owner calls reassignAuditTestDataset.
4. The outsider repeats after each reassignment.

After 3 rounds the pool is at 0.75³ ≈ 42% of its starting value (asserted below 50%), and token.balanceOf(outsider) == 0 throughout. The existing suite already has this shape as intended behavior: test_resolveDispute_upheld_returnsBondAndPenalises has the disputer resolve their own dispute with wrongK and win.

Fix

This needs a small design decision first. Options:

1. Only the owner's reveal can resolve. Make resolveTestDataDispute owner-only: the owner reveals the true K and plaintext hash, and a match clears the dispute. If the owner doesn't reveal within disputeWindowBlocks, closeExpiredDispute upholds the dispute (bond back, penalty, reassignment) instead of forfeiting the bond. This reverses today's default: silence counts against the party who holds the data.
2. Restrict who can open. Only an auditor of that batch (isBatchAuditor) can open a dispute, and disputeBondAmount gets a non-zero default so each attempt costs something.
3. Separate the key check from the content check. Commit keccak256(K) on its own, so an on-chain check can prove a revealed K is the real key. A content mismatch (a bad plaintext) still can't be proven on-chain without the data, so it needs either option 1's "owner must reveal" rule or off-chain adjudication (see No. 38 on dispute resolution).

Option 1 + option 2 closes the free-griefing path without new cryptography. Tests to add or change:

• a non-owner resolve reverts, or can't reach the "upheld" branch;
• an owner reveal with the true K clears the dispute;
• an owner who stays silent past the window loses (upheld);
• a non-batch-auditor open reverts;
• update test_resolveDispute_upheld_*, which currently encode the attack.

Size note: this contract has headroom, unlike DINTaskCoordinator (No. 201 Part A).

Related

No. 38 (dispute resolution, P3-4.3), No. 115 (encrypted test-data key assignment), No. 192 (auditor commit hash), PR No. 204.