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

Native Token Loss During Failed Withdrawals

ethworker2024

Summary

The withdrawTokens function in the Bridge.sol contract does not ensure that users' native tokens (e.g., ETH) are refunded if the withdrawal process fails. This can result in users losing their funds if the token being withdrawn is not in escrow or if there is an issue with the minting process.

Vulnerability Details

When a user initiates a token withdrawal using the withdrawTokens function, they can attach native tokens to cover gas costs. The function iterates through the token IDs in the request and attempts to withdraw each token from escrow using the _withdrawFromEscrow function. If the token is not in escrow, it tries to mint the token using the mintFromBridge function of the corresponding IERC721Bridgeable contract.

However, if either the _withdrawFromEscrow or mintFromBridge call fails for any reason, the function continues executing without refunding the user's native tokens. This means that the user's funds will be stuck in the contract, and they will not be able to recover them.

The vulnerable code is as follows:

function withdrawTokens(uint256[] calldata request) external payable returns (address) {
// ...
​
for (uint256 i = 0; i < req.tokenIds.length; i++) {
uint256 id = req.tokenIds[i];
​
bool wasEscrowed = _withdrawFromEscrow(ctype, collectionL1, req.ownerL1, id);
​
if (!wasEscrowed) {
IERC721Bridgeable(collectionL1).mintFromBridge(req.ownerL1, id);
}
}
​
// ...
}

Impact

Users may permanently lose any native tokens they attached to the withdrawal transaction if the operation fails midway. Over time, a significant amount of value could become trapped in the contract, leading to financial losses for users and damaging the protocol's reputation and trust within the community.

The severity of this vulnerability is high because it directly impacts users' funds and can result in significant financial losses.

Tools Used

  • Manual code review

  • Solidity compiler

Recommendations

To address this vulnerability, it is recommended to implement a mechanism that ensures users' native tokens are refunded if any part of the withdrawal process fails. One approach is to use a try-catch block around the escrow withdrawal and minting operations. If an exception is caught, the user's native tokens should be immediately refunded before rethrowing the error.

Here's an example of how the code can be modified to implement this fix:

function withdrawTokens(uint256[] calldata request) external payable returns (address) {
// ...
​
for (uint256 i = 0; i < req.tokenIds.length; i++) {
uint256 id = req.tokenIds[i];
​
try {
bool wasEscrowed = _withdrawFromEscrow(ctype, collectionL1, req.ownerL1, id);
​
if (!wasEscrowed) {
IERC721Bridgeable(collectionL1).mintFromBridge(req.ownerL1, id);
}
} catch {
// Refund native tokens before rethrowing the error
payable(msg.sender).transfer(msg.value);
revert("Token withdrawal failed");
}
}
​
// ...
}

By implementing this fix, the ArkProject NFT Bridge can ensure that users' funds are protected even if the withdrawal process encounters errors, enhancing the protocol's security and reliability.

Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational / Gas

Please, do not suppose impacts, think about the real impact of the bug and check the CodeHawks documentation to confirm: https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity A PoC always helps to understand the real impact possible.

Support

FAQs

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