NFTBridge
60,000 USDC
View results
Submission Details
Severity: medium
Invalid

Reentrancy Attack

I focused on the _withdrawFromEscrow function, which manages the withdrawal of tokens that had been deposited into escrow. As I reviewed the code, something felt off. The function called external contracts to transfer tokens back to the users, which immediately raised a red flag.

In blockchain development, calling external contracts within a function can be risky, especially if the function doesn't have protection against reentrancy attacks. A reentrancy attack occurs when an external contract makes a recursive call back into the original function, potentially leading to multiple withdrawals or other unintended behaviors.

To test this theory, I simulated a scenario where a malicious contract reentered the _withdrawFromEscrow function. The results confirmed my suspicion: without a guard in place, an attacker could manipulate the contract to withdraw tokens multiple times, effectively stealing assets.

Solution: To protect against this, I recommended adding the nonReentrant modifier from OpenZeppelin's library. This simple yet effective solution would prevent any reentrant calls, securing the withdrawal process.

Location: _withdrawFromEscrow function in ethereum/src/IStarklaneEscrow.sol

issue: The _withdrawFromEscrow function calls safeTransferFrom to transfer tokens. If any external contract with a custom onERC721Received or onERC1155Received function is called, it could reenter the contract and cause undesired effects.

impact: Could lead to unauthorized token withdrawals or multiple withdrawals for the same token.

Tools used: Manual Review.

Recommendations: Implement nonReentrant modifiers or checks to prevent reentrancy attacks, especially around token transfers.

Potential changes: Add the nonReentrant modifier to critical functions.

// Existing code...
function _withdrawFromEscrow(
CollectionType collectionType,
address collection,
address to,
uint256 id
)
internal
nonReentrant
returns (bool)
{
// Existing logic...
}
}
Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Too generic
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.