ArkProject: NFT Bridge

Note

The current issue is inspired by a similar finding in a previous audit, as described here, which was categorized as a valid High severity issue.

Summary

In the Starklane::depositTokens function, the TokenUtil.detectInterface utility incorrectly prioritizes the detection of ERC721 over ERC1155 when checking for token standards. Since some NFTs, including popular ones like Sandbox, support both ERC721 and ERC1155, this leads to a scenario where tokens are misclassified as ERC721 even when they might be transferred as ERC1155.

Consequently, the system incorrectly assumes that the supply for a tokenId is 1, leading to potential overwriting of escrow data if multiple users deposit tokens with the same tokenId. This results in permanent locking of tokens within the Starklane contract, leading to a loss of funds.

Vulnerability Details

The TokenUtil.detectInterface function is responsible for determining whether a given token supports the ERC721 or ERC1155 standard. However, it prioritizes the detection of ERC721, which causes issues when dealing with tokens that support both standards.

In this code, if the token supports both ERC721 and ERC1155, it will be classified as ERC721 due to the early return. This is problematic because the contract assumes that the supply for such tokens is 1, which is not necessarily true for ERC1155 tokens.

This misclassification leads to problems in the _depositIntoEscrow function, where the _escrow[collection][id] mapping is updated to store the address of the depositor. The code does not check whether the token has already been escrowed, assuming it has not been if it’s classified as ERC721.

If a second user deposits the same tokenId, the escrow data is overwritten with the new depositor’s address. The issue is further compounded when the _withdrawFromEscrow function is called, which clears the escrow data after the first withdrawal, preventing any subsequent withdrawals of the same token.

As a result, tokens that were escrowed after the initial deposit can become permanently locked in the contract, leading to a loss of funds.

Impact

Tokens that support both ERC721 and ERC1155 standards may be classified as ERC721, leading to incorrect handling of their supply and overwriting of escrow data. This can cause tokens to be permanently locked within the Starklane contract, resulting in significant loss of funds.

Tools Used

Manual

Recommendations

To prevent this issue, the ERC1155 check should be performed before the ERC721 check in the TokenUtil.detectInterface function.