sherlock-audit / 2023-12-arcadia-judging

18 stars 15 forks source link

rvierdiiev - AbstractStakingAM allows owner of nft to withdraw balance before selling nft #28

Closed sherlock-admin closed 8 months ago

sherlock-admin commented 9 months ago

rvierdiiev

high

AbstractStakingAM allows owner of nft to withdraw balance before selling nft

Summary

AbstractStakingAM allows owner of nft to withdraw balance before selling nft. This is because there is no any restrictions that exist inside vault.

Vulnerability Detail

When user locks LP tokens in the AbstractStakingAM, then erc721 token is created for him. Later user can do whatever he wants with this erc721 token. So it's possible that such tokens will be traded on markets.

AbstractStakingAM.decreaseLiquidity function decreases position balance for the nft. Currently there is no guarantee for the purchaser, that he will receive nft with LP amount that he wanted to buy as nft owner can decrease almost whole position right before order will be filled in the nft marketplace.

Impact

Nft purchaser can get less amount of LP.

Code Snippet

Provided above

Tool used

Manual Review

Recommendation

Inside Vault contract you have already implemented protection with lastActionTimestamp variable. You can reuse it here as well. Or you can allow to use StakingAM.mint to put token as collateral only. In this case function should send token to the vault after creation and when withdraw from vault it should unstake fully.

sherlock-admin2 commented 8 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid

nevillehuang commented 8 months ago

Invalid, I believe it is buyer and sellers own responsibility to execute sale of NFTs given it is not mentioned publicly/in contest details that the NFT is tradeable