Skip to content

fix(migration): forbid emergencyWithdraw of the migration token#189

Merged
drewstone merged 1 commit into
mainfrom
fix/migration-emergency-withdraw-token-guard
Jun 24, 2026
Merged

fix(migration): forbid emergencyWithdraw of the migration token#189
drewstone merged 1 commit into
mainfrom
fix/migration-emergency-withdraw-token-guard

Conversation

@drewstone

Copy link
Copy Markdown
Contributor

What

TangleMigration.emergencyWithdraw now reverts (EmergencyWithdrawTokenForbidden) when _token is the migration token. The migration asset can only ever leave the contract via the permissionless sweepUnclaimedToTreasury → immutable treasury. Foreign-token and ETH rescue is unchanged.

Why

emergencyWithdraw(_token, _amount) is onlyOwner and sends to owner() (transferable via Ownable). It had no guard against _token == address(token), so once paused or past the claim deadline the owner could pull the migration token itself — i.e. unclaimed user allocations — straight to owner().

That bypassed the entire trustless design: treasury is immutable and sweepUnclaimedToTreasury() is callable by anyone after the deadline, specifically so unclaimed funds can only land in a fixed treasury and no privileged actor can redirect them. emergencyWithdraw silently reintroduced full owner discretion over those same funds.

In production owner will be a multisig Safe, which lowers exploitability to "signers collude / Safe compromised" — but the guard makes the code match the promise the contract advertises to users, rather than relying on the signers' good behavior. Cheap, no downside.

This is audit finding #7 (graded medium). The originally-proposed remediation ("route emergencyWithdraw to treasury") was rejected because it would break the legitimate rescue of mis-sent foreign tokens; guarding the migration token specifically closes the hole without that cost.

Tests

Replaced the prior test that withdrew the migration token to owner (the buggy behavior) with:

  • test_EmergencyWithdraw_RevertsForMigrationToken — migration token is forbidden even when paused
  • test_EmergencyWithdraw_RescuesForeignToken — a foreign token is still rescuable to owner
  • test_EmergencyWithdraw_RevertIfNotPausedOrDeadlinePassed — paused/deadline gate still enforced (via a foreign token)

forge test --match-path test/TangleMigration.t.sol41 passed, 0 failed.

emergencyWithdraw sent any token to owner() (transferable via Ownable),
including the migration token itself once paused or past the claim
deadline. That bypassed sweepUnclaimedToTreasury — the permissionless
path that routes unclaimed funds to the immutable treasury — letting the
owner reclaim user-claimable funds and defeating the guarantee the
immutable treasury + anyone-can-sweep design exists to provide.

Guard emergencyWithdraw against _token == address(token); the migration
asset may only exit via sweepUnclaimedToTreasury. Foreign-token/ETH
rescue is unchanged.

Closes audit finding #7 (medium).

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved PR — 66e0b495

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-24T18:23:08Z

@drewstone drewstone merged commit 8d7e6fc into main Jun 24, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants