69836 sc low setmigrationpermit blocks users from revoking permits after role removal stale permits auto reactivate on re grant and drain user funds

Submitted on Mar 17th 2026 at 02:36:26 UTC by @raphaelbgr for Audit Comp | Folks Finance: Staking Contracts

  • Report ID: #69836

  • Report Type: Smart Contract

  • Report severity: Low

  • Target: https://github.com/Folks-Finance/folks-staking-contracts/blob/main/src/Staking.sol

  • Impacts:

    • Theft of unclaimed yield

Description

Title

setMigrationPermit blocks users from revoking permits after role removal -- stale permits auto-reactivate on re-grant and drain user funds


Brief/Intro

so setMigrationPermit checks hasRole for both granting AND revoking. when admin revokes MIGRATOR_ROLE from some migrator address, any user who previously permitted that address cant revoke their permit anymore. the call just reverts with MigratorNotFound. permit stays true in storage, no way to clean it up.

the thing is, if that role ever gets re-granted to the same address (and it will -- protocol upgrades, incident recovery, V2 to V3 migration, whatever), every single stale permit reactivates silently. migrator calls migratePositionsFrom and drains the users new stakes without asking anyone. cascade PoC shows 3 users drained for 17,600 FOLKS from a single re-grant.

this is not known issue #12.

Vulnerability Details

look at src/Staking.sol, setMigrationPermit, line 78:

that role check on line 78 doesnt care if youre granting or revoking. for granting yeah it makes sense, you shouldnt permit a random address. but for revoking? the user is just removing their own authorization. that should always work, doesnt matter if the target still has the role or not.

so when admin revokes the role the user gets permanently locked out of cleaning their own permit. and when the role comes back the permit is just sitting there ready to go.

why this is not known issue #12:

known issue #12 says "migrationPermits may contain migrator whose MIGRATOR_ROLE was later revoked" and the mitigation is basically "without the role the permit cant be acted on." ok thats true but it misses three things:

  1. users cant clean it up. calling setMigrationPermit(migrator, false) reverts with MigratorNotFound. known issue #12 doesnt say anything about this.

  2. permits come back to life on re-grant. known issue #12 assumes the role stays revoked forever. in reality roles get re-granted all the time. the mitigation breaks the moment the role comes back.

  3. actual money gets stolen. known issue #12 treats stale entries like harmless storage junk. the cascade PoC proves they cause theft of 11,000 FOLKS per victim.

why this isnt privileged role abuse:

granting revoking and re-granting roles is normal admin stuff. the bug is that the contract makes user permissions irrevocable and auto-reactivating. thats a design flaw not role abuse.

fix: add one condition to line 78:

revoking always works now. granting still checks the role. done.

Impact Details

heres how the cascade plays out: user permits migrator, admin revokes role, user tries to revoke permit but gets blocked, user stakes new tokens thinking theyre safe, admin re-grants role for whatever reason, migrator drains everything.

at 10% APR for 365 days:

  • 1,000 FOLKS staked = 1,100 FOLKS stolen

  • 10,000 FOLKS staked = 11,000 FOLKS stolen

  • 100,000 FOLKS staked = 110,000 FOLKS stolen

multi-victim: one re-grant drains everyone who ever permitted that migrator. cascade PoC shows 3 users losing 17,600 FOLKS in one go.

user has zero defense. their only cleanup function is broken for exactly this scenario.

References

  • src/Staking.sol line 78 (role check in setMigrationPermit)

  • src/Staking.sol line 172 (migratePositionsFrom reads the permit)

  • known issue #11 (role persistence -- different storage different fix)

  • known issue #12 (stale entries -- incomplete mitigation, see above)

Proof of Concept

Steps to reproduce

  1. deploy staking contract, add a staking period, grant MIGRATOR_ROLE to a migrator address

  2. alice approves tokens and calls setMigrationPermit(migrator, true)

  3. alice stakes 1,000 FOLKS

  4. admin revokes MIGRATOR_ROLE from migrator

  5. alice calls setMigrationPermit(migrator, false) -- reverts with MigratorNotFound

  6. alice stakes 10,000 FOLKS (new tokens, post-revocation)

  7. admin re-grants MIGRATOR_ROLE to migrator

  8. migrator calls migratePositionsFrom(alice) -- succeeds, drains 11,000 FOLKS

  9. alice received nothing, never consented

Command

also the standalone test:

PoC source code

https://gist.github.com/raphaelbgr/43fc2a07221d2a0e32915b4cc821ed3b

https://gist.github.com/raphaelbgr/43fc2a07221d2a0e32915b4cc821ed3b

Proof of Concept

cascade PoC showing multi-victim impact:

Was this helpful?