Pieces Protocol

First Flight #32
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Severity: medium
Invalid

Contradictory Logic in divideNft Function

Summary

The following line of code in the divideNft function contradicts the logic enforced by the onlyNftOwner modifier:

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

The onlyNftOwner modifier ensures that the caller (msg.sender) owns the NFT before executing the function. However, the line above rechecks ownership after the NFT has already been transferred to the contract, expecting the caller to no longer own the NFT. This creates a logical contradiction:

  • Before this line, the onlyNftOwner modifier has already validated that msg.sender owns the NFT.

  • This line assumes the ownership should have already transferred, but it also unnecessarily reverts if the caller still owns the NFT.

Impact

Unnecessary Reversion:

  • If the NFT transfer fails, the function reverts without a valid explanation of why the transfer failed.

  • This can create confusion as the revert condition is misleading—ownership verification has already been performed by the onlyNftOwner modifier.

Tools Used

Manual review

Recommendations

Replace the contradictory line with a direct validation of the transfer, ensuring proper handling of failures or simply remove it, as the modifier has done the validation checking

if (IERC721(nftAddress).ownerOf(tokenId) != address(this)) {
revert TokenDivider__NftTransferFailed();
}
Updates

Lead Judging Commences

fishy Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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