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

Reentrancy Vulnerability in `Escrow::_withdrawFromEscrow` Function which allows for a potential reentrant call back into the withdrawTokens function

bhanujangid

Description:

The Escrow::_withdrawFromEscrow function contains a reentrancy vulnerability due to the order of operations in the code. Specifically, the function performs an external call to transfer tokens (safeTransferFrom) before updating the escrow state. This allows for a potential reentrant call back into the Bridge::withdrawTokens function, which could result in unauthorized withdrawals or double-spending of tokens.

Impact:

If exploited, this vulnerability could allow an attacker to perform reentrant calls during the token withdrawal process. As a result, the attacker might withdraw the same token multiple times, leading to unauthorized access to tokens and possible loss of assets within the contract.

Proof of Concept:

In this scenario, the MaliciousContract re-enters the Bridge::withdrawTokens function during the safeTransferFrom execution, exploiting the unupdated escrow state to perform unauthorized withdrawals.

contract MaliciousContract is IERC721Receiver {
BridgeContract public bridge;
uint256[] public request; // Serialized request
​
constructor(BridgeContract _bridge, uint256[] memory _request) {
bridge = _bridge;
request = _request;
}
​
// This function is called during the safeTransferFrom call
function onERC721Received(address operator, address from, uint256 tokenId, bytes calldata data) external override returns (bytes4) {
// Re-enter the bridge contract's withdrawTokens function
bridge.withdrawTokens(request);
return this.onERC721Received.selector;
}
}

Recommended Mitigation:

  1. To prevent reentrancy update state before external call - Modify the Escrow::_withdrawFromEscrow function to update the escrow state (i.e., _escrow[collection][id] = address(0x0)) before making the external safeTransferFrom call.

function _withdrawFromEscrow(
CollectionType collectionType,
address collection,
address to,
uint256 id
)
internal
returns (bool)
{
if (!_isEscrowed(collection, id)) {
return false;
}
​
// Update state before making the external call
_escrow[collection][id] = address(0x0);
​
address from = address(this);
​
if (collectionType == CollectionType.ERC721) {
IERC721(collection).safeTransferFrom(from, to, id);
} else {
// TODO: Check here if the token supply is currently 0.
IERC1155(collection).safeTransferFrom(from, to, id, 1, "");
}
​
return true;
}

2.Use a "reentrancyGuard" - Add the nonReentrant modifier from OpenZeppelin’s library to the Bridge::withdrawTokens function to prevent reentrant calls.

function withdrawTokens(
uint256[] calldata request
)
external
payable
nonReentrant // Prevent reentrancy
returns (address)
{
// Function logic here...
}
Updates

Lead Judging Commences

n0kto Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Lack of quality
Assigned finding tags:

finding-withdraw-reentrancy-creates-unbridgeable-tokens

Impact: - NFT already bridged won’t be bridgeable anymore without being stuck. Likelyhood: Low. - Attackers will corrupt their own tokens, deploying a risky contract interacting with an upgradable proxy. They have to buy and sell them without real benefits, except being mean. Some really specific and rare scenario can also trigger that bug.

Support

FAQs

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