Pieces Protocol
First Flight #32
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Invalid

Incorrect check for NFT owner in function divideNft

hawks

Summary

Function divideNft is external that mints and transfer the NFT from msg.sender to the contract.

However, the ownership validation after the NFT transfer contains a logical error, which could lead to incorrect behavior.

Vulnerability Details

After transferring the NFT, the function attempts to check whether the transfer was successful by verifying the owner of the NFT. The condition will only trigger the rever if sender still owns the NFT.

It should instead check that the contract itself is the new owner of the NFT, which is incorrect.

​
if(IERC721(nftAddress).ownerOf(tokenId) == msg.sender)
{ revert TokenDivider__NftTransferFailed(); }

Impact

Loss of funds or NFT locking

Tools Used

Manual review

Recommendations

Change the condition to verify that the contract is the new owner of the NFT:

​
- if(IERC721(nftAddress).ownerOf(tokenId) == msg.sender)) {
​
+ if(IERC721(nftAddress).ownerOf(tokenId) != address(this)) {
​
if(IERC721(nftAddress).ownerOf(tokenId) != address(this)) { revert TokenDivider__NftTransferFailed(); }

Updates

Lead Judging Commences

fishy Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.