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

Reentrancy attack in StarklaneEscrow::_withdrawFromEscrow function, this can lead to draining of assets/funds from the protocol

emma77

Summary

Reentrancy attack in StarklaneEscrow::_withdrawFromEscrow function because State variables written after the call(s)

Vulnerability Details

Reentrancy vulnerability, State variables written after the call(s)

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); //@audit --> Reentrancy attack?
// TODO:
// Check here if the token supply is currently 0.
IERC1155(collection).safeTransferFrom(from, to, id, 1, "");
}
​
@> _escrow[collection][id] = address(0x0);
​
return true;
}
​
​

Impact

Reentrancy attack that can lead to draining of assets/funds from the protocol

Tools Used

manual review

Recommendations

Implement the CEI pattern or
Reentrancy Guards

Example: The function can be written

function _withdrawFromEscrow(
CollectionType collectionType,
address collection,
address to,
uint256 id
)
internal
returns (bool)
{
if (!_isEscrowed(collection, id)) {
return false;
}
+ _escrow[collection][id] = address(0x0);
address from = address(this);
​
if (collectionType == CollectionType.ERC721) {
IERC721(collection).safeTransferFrom(from, to, id); //@audit --> Reentrancy attack?
// TODO:
// Check here if the token supply is currently 0.
IERC1155(collection).safeTransferFrom(from, to, id, 1, "");
}
- _escrow[collection][id] = address(0x0);
return true;
}
Updates

Lead Judging Commences

n0kto Lead Judge about 1 year 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.