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

Missing Ownership Check of token in `depositTokens` Function

0xrakesh_ummadi28

Summary

The depositTokens function in the Starklane contract lacks an ownership check for ERC721 tokens. This omission allows users to deposit tokens they do not own, leading to potential unauthorized transfers.

FIle name : Bridge.sol

Vulnerability Details

Location : https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/Bridge.sol#L120

The depositTokens function processes token deposits without verifying the ownership of ERC721 tokens. This oversight means that the function can be exploited to deposit tokens from other users' accounts if proper ownership checks are not enforced.

Impact

  1. Users can deposit tokens they do not own, resulting in unauthorized transfers.

  2. Potential for exploitation and financial loss due to unauthorized actions.

  3. The contract’s integrity and trustworthiness are compromised, leading to potential disruption of service and loss of user confidence.

Tools Used

Maual code review

Recommendations

Add verification to ensure that the caller is the owner of ERC721 tokens before allowing deposit.

​
if (ctype == CollectionType.ERC721) {
​
//Add below check
(+) for (uint256 i = 0; i < ids.length; i++) {
| address tokenOwner = IERC721(collectionL1).ownerOf(ids[i]);
| if (tokenOwner != msg.sender) {
| revert NotOwnerError();
| }
(+) }
(req.name, req.symbol, req.uri, req.tokenURIs) = TokenUtil.erc721Metadata(
collectionL1,
ids
);
}
Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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