fix(migration): bind vesting delegatee to beneficiary to stop front-run capture

[drewstone]

What

TNTVestingFactory.getOrCreateVesting no longer accepts a caller-supplied delegatee; it forces delegatee == beneficiary. Deploy scripts and FullDeploy updated to the 3-arg signature.

Why

The delegatee was not part of the CREATE2 salt (keccak256(token, beneficiary, startTimestamp)). Since getOrCreateVesting is permissionless, a front-runner could pre-create a claimant's vesting clone at the deterministically-predicted address with an attacker delegatee. The migration's _executeClaim then found the clone already deployed, skipped initialization, and merely funded it — redirecting the beneficiary's vested voting power to the attacker (for an ERC20Votes token such as the production TangleToken).

Tokens were never at risk — beneficiary is salt-bound, so the attacker can't steal the vested tokens. Only governance voting power was redirected, and only until the beneficiary noticed and re-delegated. Real medium-severity governance griefing, especially at TGE scale across many claimants.

Binding the delegatee to beneficiary removes the attacker-controlled degree of freedom: any pre-created clone is now byte-identical to the one the migration would create, so a front-run is a harmless no-op. The beneficiary may still re-delegate afterward via the vesting contract.

How found

Red-team exploit PoC. The PoC is retained as a passing regression test: the attacker pre-creates the clone, but after the victim's claim the attacker holds 0 votes and the beneficiary holds all of the vested voting power.

Verification

• packages/migration-claim: 42/42 tests pass (forge test), including the new regression.
• Main repo compiles clean with the FullDeploy 3-arg fix.

Scope note

The two getOrCreateVesting deploy-script call sites (treasury / foundation vesting) already passed delegatee == recipient, so dropping the argument is behavior-preserving.

[tangletools]

✅ Auto-approved PR — 6cf4c50f

Blanket team auto-approval is enabled for this reviewer service.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

_{tangletools · auto-approval · reason: blanket_auto_approve · 2026-06-24T19:45:32Z}

[tangletools]

🟡 Value Audit — sound-with-nits

Verdict	sound-with-nits
Concerns	2 (1 low, 1 weak-concern)
Heuristic	0.0s
Duplication	0.0s
Interrogation	89.9s (2 bridge agents)
Total	89.9s

💰 Value — sound-with-nits

Binds vesting delegatee to beneficiary to close a front-run voting-power hijack; good fix, but diverges from the existing TNTLockFactory pattern that solves the same clone-delegation problem differently.

• What it does: Removes the caller-supplied delegatee parameter from TNTVestingFactory.getOrCreateVesting and forces initialization with delegatee == beneficiary. Updates all call sites (TangleMigration.sol:415, DeployTangleMigration.s.sol:194/222, FullDeploy.s.sol:1646/1667) to the 3-arg signature and adds a regression test proving a front-runner can pre-create the clone but cannot capture voting pow
• Goals it achieves: Prevents a permissionless front-runner from pre-creating a claimant's deterministic vesting clone with an attacker-chosen delegatee, which would otherwise redirect the beneficiary's vested ERC20Votes voting power to the attacker. Restores the invariant that vested voting power starts with the beneficiary.
• Assessment: Good change. The vulnerability is real, the fix is minimal and coherent, the regression test directly exercises the exploit path, and the deploy-script call sites are behavior-preserving because they already passed delegatee == recipient.
• Better / existing approach: The repo already has an analogous permissionless deterministic clone factory — src/governance/lockups/TNTLockFactory.sol / TNTCliffLock.sol — that closes the same delegation-hijack vector by keeping the delegatee argument for ABI/event compatibility but never auto-delegating at init (TNTCliffLock.sol:48-57, with regression tests in test/security/TNTLockupAuditPoC.t.sol:62-83). The PR cho
• Model: opencode/kimi-for-coding/k2p7
• Bridge attempts: 1

🎯 Usefulness — sound

A minimal, correct fix that removes a caller-controlled delegatee so a front-run pre-creation becomes a harmless no-op; every existing call site already passed delegatee==recipient, so it is behavior-preserving and fully wired.

• Integration: Fully reachable. The live runtime caller is TangleMigration._executeClaim (TangleMigration.sol:415), invoked by the user-facing claim flow (claimWithZKProof et al.). All five production call sites — two in script/FullDeploy.s.sol (1646, 1667), two in packages/migration-claim/script/DeployTangleMigration.s.sol (194, 222), and the migration itself — are updated to the 3-arg signature, matching the f
• Fit with existing patterns: Fits the established factory + EIP-1167 clone + CREATE2 get-or-create pattern exactly; the change only drops one parameter and hardcodes it to beneficiary inside the factory (TNTVestingFactory.sol:78). The salt (_computeSalt, line 120) was already (token, beneficiary, startTimestamp), so the fix aligns the delegatee with what the salt already bound — no new pattern, no competing abstraction. READM
• Real-world viability: Holds under the adversarial path it targets: because the delegatee is now a deterministic function of the salt-bound beneficiary, an attacker pre-creating the clone produces a byte-identical contract, so the migration's get-or-create branch funds the victim's own clone — verified by the regression test asserting attacker votes == 0 and victim votes == vestedAmount (Exploit_...t.sol:121-125). Benef
• Model: opencode/zai-coding-plan/glm-5.2
• Bridge attempts: 1

🔎 Heuristic Signals

🟡 Cruft: magic number added packages/migration-claim/test/exploit/Exploit_MigrationClaim_VestingDelegateeFrontRun.t.sol

• uint256 constant TEST_AMOUNT = 1000 ether;

💰 Value Audit

🟡 Vesting factory duplicates a solved clone-delegation pattern with a different invariant [against-grain] ``

src/governance/lockups/TNTLockFactory.sol already solved the same permissionless-deterministic-clone + delegation-hijack problem by never auto-delegating at creation (TNTCliffLock.sol:48-57). This PR instead forces delegatee == beneficiary. Both work, but the codebase now has two factories with two different invariants for the same security property. Consider aligning them — either adopt the no-auto-delegate model here, or document why the vesting factory intentionally diverges.

---

What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass	What it asks
Heuristic	Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication	Do added function/class names already exist elsewhere in the repo?
Value Audit	What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit	Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

_{value-audit · 20260624T203052Z}

[tangletools]

✅ No Blockers — 6cf4c50f

Readiness 77/100 · Confidence 80/100 · 7 findings (7 low)

	opencode-kimi	glm	deepseek	aggregate
Readiness	95	77	92	77
Confidence	80	80	80	80
Correctness	95	77	92	77
Security	95	77	92	77
Testing	95	77	92	77
Architecture	95	77	92	77

Full multi-shot audit completed 4/4 planned shots over 5 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 4/4 planned shots over 5 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 4/4 planned shots over 5 changed files. Global verifier still owns final merge decision.

🟡 LOW Deploy script treasury/foundation vesting paths lack direct test coverage — packages/migration-claim/script/DeployTangleMigration.s.sol

Lines 187-235 (treasury + foundation vesting creation via getOrCreateVesting) are exercised only indirectly through the factory's own tests and the exploit regression; no forge test actually runs DeployTangleMigration.run() with TREASURY_AMOUNT/FOUNDATION_AMOUNT set to assert the vesting clones are created and funded. This is a pre-existing gap, not introduced by this diff (the diff only deletes a redundant arg). Impact: a future regression in the script's vesting wiring could ship unobserved. Fix (optional, out of this shot's scope): add a deploy-script integration test that sets the env vars and asserts balances/vesting clone ex

🟡 LOW Live deployments past first claim cannot adopt the fix (VestingConfigLocked) — packages/migration-claim/src/TangleMigration.sol

setVestingConfig (TangleMigration.sol:309-323) reverts with VestingConfigLocked once totalClaimed!=0. The factory address is immutable per-claim (read at _executeClaim:415), so any migration instance that has already processed a claim keeps the vulnerable 4-arg factory and remains exploitable. This is a deployment-timing concern, not a code bug: the fix only protects instances upgraded before first claim or redeployed. Confirm with the team that no mainnet migration instance has live claims with the old factory; if one does, it needs a fresh deployment (claims mapping re-protection) rather than an in-place upgrade.

🟡 LOW Misleading comment wording in @dev NatSpec — packages/migration-claim/src/lockups/TNTVestingFactory.sol

Line 60: 'any pre-created clone is byte-identical to the one this call would create, so front-running is a harmless no-op.' This is technically true for EIP-1167 clones (they always are byte-identical since they point to the same implementation) but it's not the mechanism of protection. The actual protection is that initialize() is now called with delegatee=beneficiary regardless of caller, so the attacker cannot inject a different delegatee. The comment could mislead a reader into thinking bytecode identity is the defense when it's the deterministic initialization state. No functional impact. Suggested reword: 'the factory now forces de

🟡 LOW TNTLinearVesting.initialize keeps a delegatee param with no onlyFactory guard (pre-existing, not closed by this fix) — packages/migration-claim/src/lockups/TNTVestingFactory.sol

The factory now always passes beneficiary as delegatee (TNTVestingFactory.sol:78), so the sanctioned path is safe. But TNTLinearVesting.initialize (TNTLinearVesting.sol:80-107) still accepts an arbitrary delegatee argument and has no access control (only an AlreadyInitialized re-entrancy guard). It is only safe today because cloneDeterministic+initialize are atomic within getOrCreateVesting and no one else can replicate the factory's CREATE2 sender address. If a future change ever separates clone creation from initialization, or adds a second factory path, an attacker could initialize a fresh clone with a malicious delegatee before the factory does. Consider adding an onlyFactory (or initializer-origin) guard to TNTLinearVesting.initialize so the contract defends itself rather than relying

🟡 LOW Missing trailing newline at end of file — packages/migration-claim/test/exploit/Exploit_MigrationClaim_VestingDelegateeFrontRun.t.sol

The diff shows \ No newline at end of file after line 129. forge fmt convention expects a trailing newline. Add a blank line at EOF.

🟡 LOW Unused console2 import — packages/migration-claim/test/exploit/Exploit_MigrationClaim_VestingDelegateeFrontRun.t.sol

Line 4 imports {Test, console2} but console2 is never used in the file. forge fmt or a linter may flag this. Fix: change to import {Test} from "forge-std/Test.sol";

🟡 LOW Deploy-script change has no direct test coverage — script/FullDeploy.s.sol

Lines 1646 and 1667 are updated to the 3-arg signature, but the regression test at packages/migration-claim/test/exploit/Exploit_MigrationClaim_VestingDelegateeFrontRun.t.sol calls the factory directly, not via FullDeploy. Since both sites already passed delegatee==beneficiary under the old signature, the functional delta is zero and the risk is correspondingly nil — but there is no forge script dry-run (e.g., forge script --simulate) asserted in CI for this path. Acceptable given the behavior-preserving nature; flagged only for completeness.

---

_{tangletools · 2026-06-24T20:46:10Z · trace}

[tangletools]

✅ Approved — 7 non-blocking findings — 6cf4c50f

Full multi-shot audit completed 4/4 planned shots over 5 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 4/4 planned shots over 5 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 4/4 planned shots over 5 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary

---

_{tangletools · 2026-06-24T20:46:10Z · immutable trace}