69524 sc low role validation on revocation can lock migration permits
Submitted on Mar 15th 2026 at 12:15:38 UTC by @theboiledcorn for Audit Comp | Folks Finance: Staking Contracts
- Report ID: #69524
- Report Type: Smart Contract
- Report severity: Low
- Target: https://github.com/Folks-Finance/folks-staking-contracts/blob/main/src/Staking.sol
- Impacts:
- Contract fails to deliver promised returns, but doesn't lose value
Description
Brief/Intro
The setMigrationPermit function in the Staking contract allows users to authorize or revoke a migrator address that can migrate their staking positions. However, the implementation enforces that the specified migrator must currently hold the MIGRATOR_ROLE for both granting and revoking permissions.
If the role is later removed from the migrator by an administrator, users who previously granted a permit to that address lose the ability to revoke it. Any attempt to call setMigrationPermit(migrator, false) reverts with MigratorNotFound.
As a result, the stored permit remains permanently set to true with no direct on-chain method for the user to clear it. This behavior conflicts with the documented guarantee that migration permissions “can be revoked at any time.”
Vulnerability Details
function setMigrationPermit(address _migrator, bool _isMigrationPermitted) external {
if (!hasRole(MIGRATOR_ROLE, _migrator)) revert MigratorNotFound(_migrator);
migrationPermits[_migrator][msg.sender] = _isMigrationPermitted;
emit MigrationPermitUpdated(_migrator, msg.sender, _isMigrationPermitted);
}
The role verification is applied regardless of whether the user is granting (true) or revoking (false) permission.
When the migrator’s role has already been revoked, the hasRole check fails and the transaction reverts. This prevents users from removing previously granted authorization.
Scenario
- Alice grants a migration permit to
migratorwhile the address holds theMIGRATOR_ROLE. - At a later time, an admin removes the migrator’s role.
- Alice attempts to revoke the authorization by calling
setMigrationPermit(migrator, false). - The transaction reverts because the migrator no longer satisfies the
MIGRATOR_ROLErequirement.
The permit therefore remains stored as true. Alice has no way to clear this entry through the contract interface.
Although the migrator cannot execute migrations while the role is absent, the permit effectively persists indefinitely. If the role is granted again in the future, the authorization automatically becomes active again without Alice explicitly re-approving it.
Impact Details
- Irrevocable Authorization: Users may be unable to revoke migration permissions that were previously granted, contradicting documented behavior.
- Persistent Latent Permissions: Old approvals remain recorded even after a migrator loses its role.
- Potential Future Risk: If the same address regains the
MIGRATOR_ROLElater, all existing permits instantly become valid again without user interaction.
There is no immediate loss of funds, since migration calls still require the MIGRATOR_ROLE. However, the inability to revoke approvals breaks expected permission management and might introduce long-term authorization risk.
References
Recommended Mitigation
The role check should only apply when granting a migration permit. Revocation should always be allowed, regardless of the migrator’s current role status.
function setMigrationPermit(address _migrator, bool _isMigrationPermitted) external {
if (_isMigrationPermitted && !hasRole(MIGRATOR_ROLE, _migrator)) {
revert MigratorNotFound(_migrator);
}
migrationPermits[_migrator][msg.sender] = _isMigrationPermitted;
emit MigrationPermitUpdated(_migrator, msg.sender, _isMigrationPermitted);
}
This modification ensures that:
- New permits can only be granted to valid migrators.
- Users retain the ability to revoke previously granted permissions at any time.
Proof of Concept
Insert into Staking.t.sol
function test_UserCannotRevokePermitIfMigratorRoleRemoved() public {
// fund contracts and users
deal(address(token), address(staking), 1_000 ether);
deal(address(token), alice, 100 ether);
// create staking configuration
uint8 pid = addStakingPeriodByManager(50 ether, 20, 10, 5000, true);
// alice stakes
approveAndStake(alice, pid, 10 ether, 20, 10, 5000, address(0));
// alice authorizes migrator
vm.startPrank(alice);
staking.setMigrationPermit(migrator, true);
vm.stopPrank();
assertEq(staking.migrationPermits(migrator, alice), true);
// store role identifier
bytes32 role = staking.MIGRATOR_ROLE();
// admin removes migrator privileges
vm.prank(admin);
staking.revokeRole(role, migrator);
assertFalse(staking.hasRole(role, migrator));
// alice now attempts to disable the permit
vm.startPrank(alice);
vm.expectRevert(
abi.encodeWithSelector(IStakingV1.MigratorNotFound.selector, migrator)
);
staking.setMigrationPermit(migrator, false);
vm.stopPrank();
// the approval is stuck as true
bool stillApproved = staking.migrationPermits(migrator, alice);
assertTrue(stillApproved);
}