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

0 stars 0 forks source link

Incorrect Implementation of ERC721 Safe Transfer Check #193

Closed c4-bot-2 closed 1 month ago

c4-bot-2 commented 1 month ago

Lines of code

https://github.com/superposition-finance/superposition-core/blob/main/pkg/sol/OwnershipNFTs.sol#L92-L95

Vulnerability details

Vulnerability detail

The _onTransferReceived function in the OwnershipNFTs contract incorrectly implements the check for the onERC721Received selector. The function reverts when it receives the correct selector and allows the transfer for any other value, which is the opposite of the intended behavior.

Impact

This vulnerability can lead to several severe consequences:

Proof of Concept

require(
    data != IERC721TokenReceiver.onERC721Received.selector,
    "bad nft transfer received data"
);

This code checks if the returned data is NOT equal to the expected selector, which is the opposite of the correct implementation.

Tools Used

Manual code review

Recommended Mitigation Steps

The check should be corrected to verify that the returned data matches the expected selector:

require(
    data == IERC721TokenReceiver.onERC721Received.selector,
    "bad nft transfer received data"
);

Assessed type

Invalid Validation