ArkProject: NFT Bridge

Summary

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

The StarklaneEscrow contract fails to implement a necessary supply check for ERC1155 tokens during the deposit and withdrawal processes. This oversight could lead to the mishandling of tokens that have a supply greater than one, potentially causing financial loss or incorrect state management.

Vulnerability Details

The StarklaneEscrow contract is designed to manage the escrow of both ERC721 and ERC1155 tokens. However, there is a critical vulnerability in the handling of ERC1155 tokens. The contract lacks an essential check to verify the supply of ERC1155 tokens being deposited or withdrawn.

ERC1155 tokens can have a supply greater than one, meaning multiple instances of a token can exist. The contract assumes that the supply of any ERC1155 token being transferred is exactly one, but this assumption is not enforced by the code. This oversight can lead to the incorrect handling of tokens, where a user could potentially deposit or withdraw tokens without correctly managing their quantities

The contract code indicates a TODO comment to implement this check, but it has not been completed:

// TODO: check the supply is exactly one.
// (this is the low-level call to verify if a contract has some function).
// (but it's better to check with supported interfaces? It's 2 calls instead
// of one where we control the fail.)

Without this check, the contract could inadvertently allow the transfer of tokens with a supply greater than one, leading to discrepancies in the escrowed assets.

Steps to Reproduce

Deploy the StarklaneEscrow contract.

• Create an ERC1155 token with a supply greater than one.

• Attempt to deposit the ERC1155 token into escrow using the _depositIntoEscrow function.

• Observe that the contract does not verify the token’s supply, allowing the token to be deposited without any supply check.

• Attempt to withdraw the ERC1155 token using the _withdrawFromEscrow function.

• Note that the withdrawal process does not account for the token's supply, potentially leading to incorrect asset management

Impact

The impact of this vulnerability is significant, as it can lead to:

• Mismanagement of Escrowed Assets: Tokens with supplies greater than one may be incorrectly managed, resulting in loss or duplication of assets.

• Potential Loss of User Funds: Users may lose their tokens or have incorrect token balances due to this oversight.

• Security Risks: The escrow contract may become vulnerable to exploits that take advantage of the lack of supply checks, leading to further financial risks.

Tools Used

Manual Review

Recommendations

To mitigate this vulnerability, the following steps should be implemented:

1. Implement Supply Check: Add a check to verify that the supply of any ERC1155 token being deposited or withdrawn is exactly one. This can be done by calling the balanceOf function to ensure that the contract only handles tokens with a supply of one.

Improve Documentation: Clearly document the expected behavior for ERC1155 tokens in the contract to avoid any assumptions that could lead to future vulnerabilities.

• Add Unit Tests: Create comprehensive unit tests to ensure that the supply check works correctly and that the contract handles ERC1155 tokens as expected.