stopthecap - Users are unable to `unstake` and `emergencyUnstake`

[sherlock-admin]

stopthecap

high

Users are unable to unstake and emergencyUnstake

Summary

Users are unable to unstake NFTs and emergencyUnstake NFTs

Vulnerability Detail

when users try to unstake their NFT, the unstake and emergencyUnstake function will fail due to a missing approval to transfer the NFT from the contract to the caller. If you check the Open Zeppekin implementation:

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/1a77a508f93e2df058cb082def4753a060aefa8f/contracts/token/ERC721/ERC721.sol#L154

you can see that when calling transferFrom, it is required an approval before hand to actually transfer the NFT.

Impact

Users are unable to unstake their NFTs (stuck NFTs)

Code Snippet

https://github.com/sherlock-audit/2023-04-ajna/blob/main/ajna-core/src/RewardsManager.sol#L813

https://github.com/sherlock-audit/2023-04-ajna/blob/main/ajna-core/src/RewardsManager.sol#L216-L219

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/1a77a508f93e2df058cb082def4753a060aefa8f/contracts/token/ERC721/ERC721.sol#L154

Tool used

Manual Review

Recommendation

Approve the transfer from the NFT before actually sending it

[ith-harvey]

no requirement of approvals for unstaking

[grandizzy]

proof that report is invalid https://github.com/ajna-finance/contracts/pull/912 (assert owner before unstake is RewardsManager contract and owner after unstake is staker)

[0xffff11]

Agree with sponsor. Invalid