ArkProject: NFT Bridge
ArkProject
NFTBridge
60,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Dual-Standard NFTs (ERC721/1155) Cause NFT Transfer Issue in Escrow.sol::_withdrawFromEscrow Function

Hama

Summary

Some NFT tokens support both ERC721 and ERC1155 standards. The _withdrawFromEscrow function currently checks for ERC721 support first, which may lead to the incorrect handling of transfers for assets that are actually ERC1155 tokens, such as Sandbox's ASSETs. This results in the user receiving fewer tokens than expected in certain scenarios.

Vulnerability Details

https://github.com/Cyfrin/2024-07-ark-project/blob/main/apps/blockchain/ethereum/src/Escrow.sol#L78

function _withdrawFromEscrow(
CollectionType collectionType,
address collection,
address to,
uint256 id
)
internal
returns (bool)
{
...
if (collectionType == CollectionType.ERC721) {
IERC721(collection).safeTransferFrom(from, to, id);
} else {
// TODO:
// Check here if the token supply is currently 0.
IERC1155(collection).safeTransferFrom(from, to, id, 1, "");
}
...}

In Escrow.sol:_withdrawFromEscrow, the contract checks for ERC721 support before ERC1155. If the token supports both interfaces, it defaults to the ERC721 transfer logic, even when multiple tokens are expected, leading to an incomplete transfer.

Check the return values of Sandbox's ASSETs's supportInterface https://etherscan.io/token/0xa342f5d851e866e18ff98f351f2c6637f4478db5,
both supportInterface(0x80ac58cd) and supportInterface(0xd9b67a26) return true.

Impact

A user attempting to purchase multiple tokens may only receive one, leading to potential financial loss and disruption to the intended transaction.

Tools Used

Manual review

Recommendations

Reorder the interface checks to prioritize ERC1155 over ERC721 when handling tokens that support both standards. This ensures that the correct transfer logic is applied based on the expected quantity of tokens.

function _withdrawFromEscrow(
CollectionType collectionType,
address collection,
address to,
uint256 id
)
internal
returns (bool)
{
...
if (collectionType == CollectionType.ERC1155) {
IERC1155(collection).safeTransferFrom(from, to, id, 1, "");
} else {
IERC721(collection).safeTransferFrom(from, to, id);
​
}
...}
Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Out of scope
Assigned finding tags:

invalid-both-standard-same-nft

Great catch ! Unfortunately only ERC721 are in scope. Tokens with both standard are not supported and the collection and using it that way would be a user error.

Support

FAQs

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