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

State Inconsistency Risk in _withdrawFromEscrow Function

Bellatrix

Relevant Github link:

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

Summary:

The _withdrawFromEscrow function in the Escrow.sol contract presents a potential state inconsistency risk due to updating the contract's state after external calls.

Vulnerability Details:

In the _withdrawFromEscrow function, the contract's state is updated after making external calls to transfer NFTs. This creates a brief window where the contract's internal state does not accurately reflect the true ownership status of the NFT.

function _withdrawFromEscrow(
CollectionType collectionType,
address collection,
address to,
uint256 id
)
internal
returns (bool)
{
if (!_isEscrowed(collection, id)) {
return false;
}
address from = address(this);
if (collectionType == CollectionType.ERC721) {
IERC721(collection).safeTransferFrom(from, to, id); // External call
} else {
IERC1155(collection).safeTransferFrom(from, to, id, 1, ""); // External call
}
_escrow[collection][id] = address(0x0); // State update after external call
return true;
}

Impact:

It could potentially lead to:

  • Interference with other contract logic that relies on the escrow state

  • Possible griefing attacks causing confusion or disrupting normal contract operations

Tools Used:

Manual code review

Recommendations:

Implement the checks-effects-interactions pattern by updating the state before making external calls:

function _withdrawFromEscrow(
CollectionType collectionType,
address collection,
address to,
uint256 id
)
internal
returns (bool)
{
if (!_isEscrow[collection][id]) {
return false;
}
address from = address(this);
​
// Update state before external call
_escrow[collection][id] = address(0x0);
​
if (collectionType == CollectionType.ERC721) {
IERC721(collection).safeTransferFrom(from, to, id);
} else {
IERC1155(collection).safeTransferFrom(from, to, id, 1, "");
}
​
return true;
}
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.