ArkProject: NFT Bridge

Summary

The _depositIntoEscrow function in the Starklane escrow contract does not verify the success of ERC721 token transfers, which could lead to tokens being lost or the escrow state becoming inconsistent.

Vulnerability Details

In the _depositIntoEscrow function (lines 26-53), the ERC721 token transfer is performed without checking its return value:

The transferFrom function of some ERC721 implementations might not revert on failure but instead return a boolean indicating the success of the operation. By not checking this return value, the contract assumes the transfer was successful and updates the escrow state regardless of the actual outcome.

Impact

This vulnerability could lead to:

1. Loss of user tokens if the transfer fails silently

2. Inconsistent escrow state where the contract believes it holds tokens it doesn't actually possess

3. Potential blocking of the bridging process for affected tokens

4. Financial losses for users who believe their tokens are safely escrowed

5. The severity is high due to the potential for direct financial loss and system state inconsistency.

Tools Used

Manual code review

Recommendations

Use OpenZeppelin's safeTransferFrom function instead of transferFrom for ERC721 tokens:

or instead you can decide to still use transferFrom but check the return value.