Blocklisted or paused state in staking token can prevent service owner from unstaking

[c4-bot-4]

Lines of code

https://github.com/code-423n4/2024-05-olas/blob/3ce502ec8b475885b90668e617f3983cea3ae29f/registries/contracts/staking/StakingBase.sol#L868

Vulnerability details

The StakingBase contract allows service owners to stake their service by transferring it to the contract via stake(). The service's multisig address is set in the ServiceInfo struct at this point and cannot be modified afterwards. When the owner wants to unstake the service via unstake(), the contract attempts to transfer any accumulated rewards to this multisig address:

File: StakingBase.sol
862:         // Transfer the service back to the owner
863:         // Note that the reentrancy is not possible due to the ServiceInfo struct being deleted
864:         IService(serviceRegistry).safeTransferFrom(address(this), msg.sender, serviceId);
865: 
866:         // Transfer accumulated rewards to the service multisig
867:         if (reward > 0) {
868:             _withdraw(multisig, reward);
869:         }

However, the protocol is meant to support reward tokens which may have a blocklist and/or pausable functionality.

If the multisig address gets blocklisted by the reward token at some point after staking or the reward token pauses transfers, the _withdraw() call in unstake() will fail. As a result, the service owner will not be able to unstake their service and retrieve it.

Impact

If a service's multisig address gets blocklisted by the reward token after the service is staked, the service owner will be permanently unable to unstake the service and retrieve it from the StakingBase contract. The service will be stuck.

If the reward token pauses transfers, the owner will not be able to unstake the service until transfers are unpaused.

Additionally, the owner will not be able to claim rewards either, since these are sent to the multisig.

Proof of Concept

1. Owner calls stake() to stake their service, passing in serviceId.
2. StakingBase contract transfers the service NFT from owner and stores the service's multisig address in ServiceInfo.
3. Reward token blocklists the multisig address.
4. Owner calls unstake() to unstake the service and retrieve the NFT.
5. unstake() attempts to transfer accumulated rewards to the now-blocklisted multisig via _withdraw().
6. The reward transfer fails, reverting the whole unstake() transaction.
7. Owner is unable to ever unstake the service. Service is stuck in StakingBase contract.

Tools Used

Manual review

Recommended Mitigation Steps

Consider implementing functionality to update the multisig address. This address should ideally be updated via the service registry to ensure it is properly deployed (seeing as it must match a specific bytecode), after which it can be updated in the staking contract by pulling it from the registry.

Alternatively and to also address the pausing scenario, unstaking could be split into two separate steps - first retrieve the service NFT, then withdraw any available rewards. This way, blocklisting or pausing would not prevent retrieval of the NFT itself.

Assessed type

Token-Transfer

[kupermind]

This is an interesting scenario. In this case forced unstake condition addition to the current unstake would be a better approach, as we are not able to change the multisig address.

[c4-sponsor]

kupermind (sponsor) acknowledged

[c4-judge]

0xA5DF marked the issue as selected for report

[c4-judge]

0xA5DF marked the issue as satisfactory

[vsharma4394]

Fixed