fix(migration): forbid emergencyWithdraw of the migration token#189
Merged
Conversation
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
approved these changes
Jun 24, 2026
tangletools
left a comment
Contributor
There was a problem hiding this comment.
✅ 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
This was referenced Jun 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
TangleMigration.emergencyWithdrawnow reverts (EmergencyWithdrawTokenForbidden) when_tokenis the migration token. The migration asset can only ever leave the contract via the permissionlesssweepUnclaimedToTreasury→ immutabletreasury. Foreign-token and ETH rescue is unchanged.Why
emergencyWithdraw(_token, _amount)isonlyOwnerand sends toowner()(transferable viaOwnable). It had no guard against_token == address(token), so oncepausedor past the claim deadline the owner could pull the migration token itself — i.e. unclaimed user allocations — straight toowner().That bypassed the entire trustless design:
treasuryisimmutableandsweepUnclaimedToTreasury()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.emergencyWithdrawsilently reintroduced full owner discretion over those same funds.In production
ownerwill 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 pausedtest_EmergencyWithdraw_RescuesForeignToken— a foreign token is still rescuable to ownertest_EmergencyWithdraw_RevertIfNotPausedOrDeadlinePassed— paused/deadline gate still enforced (via a foreign token)forge test --match-path test/TangleMigration.t.sol→ 41 passed, 0 failed.