code-423n4 / 2024-08-superposition-findings

0 stars 0 forks source link

Unauthorized NFT reclaiming due to uncleared approvals #104

Closed howlbot-integration[bot] closed 1 month ago

howlbot-integration[bot] commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-08-superposition/blob/4528c9d2dbe1550d2660dac903a8246076044905/pkg/sol/OwnershipNFTs.sol#L162 https://github.com/code-423n4/2024-08-superposition/blob/4528c9d2dbe1550d2660dac903a8246076044905/pkg/sol/OwnershipNFTs.sol#L109

Vulnerability details

The OwnershipNFTs contract implements an ERC721-compliant NFT system for tracking ownership of positions of the Seawater AMM. The contract includes standard ERC721 functions such as transferFrom(), safeTransferFrom(), and approve(). However, there is a critical issue in the implementation of the transfer functionality.

The _transfer() function, which is called by all transfer methods, does not clear the getApproved mapping for the transferred token. This oversight allows a previously approved address to retain its approval even after the token has changed ownership.

This vulnerability can be exploited by a malicious user to reclaim an NFT after transferring it to another address.

Impact

Unauthorized reclaiming of transferred NFTs leading to loss of assets for the recipients.

Proof of Concept

  1. Alice owns TokenID 1 and approves Bob to manage this token using the approve() function.
  2. Alice transfers TokenID 1 to Carol.
  3. The _transfer() function is called, which updates the ownership but doesn't clear Bob's approval.
  4. Bob, who is still approved for TokenID 1, can now call transferFrom() to move the token from Carol back to himself or any other address.

Tools Used

Manual review

Recommended Mitigation Steps

Modify the _transfer() function to clear the approval for the transferred token.

Assessed type

Other

c4-judge commented 3 weeks ago

alex-ppg marked the issue as satisfactory