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

Improper Escrow Check in Escrow::_withdrawFromEscrow function allows unauthorized withdrawals

ghufranhassan1

Summary

Improper Escrow Check in _withdrawFromEscrow allows unauthorized withdrawals.

Vulnerability Details

The _withdrawFromEscrow function in the StarklaneEscrow contract checks if a token is in escrow using the _isEscrowed function. However, this check alone is insufficient to prevent unauthorized withdrawals because it does not verify if the caller is the original depositor of the token.

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);
} else {
IERC1155(collection).safeTransferFrom(from, to, id, 1, "");
}
​
_escrow[collection][id] = address(0x0);
​
return true;
}

Impact

  • Unauthorized entities can withdraw tokens from escrow.

  • Potential loss of user assets.

This the POC which was created in Escrow.t.sol as the function testFail_withdrawNotInEscrow was designed to test the scenario where a token that is not in escrow is attempted to be withdrawn.

function testFail_withdrawNotInEscrow() public {
IERC721MintRangeFree(erc721).mintRangeFree(alice, 0, 10); //Mints tokens with IDs 0 to 9 to alice.
​
uint256[] memory ids = new uint256[](1);
ids[0] = 15; // Sets the token ID to 15, which is not minted or deposited.
​
vm.prank(alice); //Sets the next call to be made by alice.
bool wasInEscrow = escrow.withdrawFromEscrow(CollectionType.ERC721, erc721, bob, ids[0]); //Attempts to withdraw token ID 15 from escrow to bob.
assertTrue(wasInEscrow); //Asserts that wasInEscrow is true.
}

The output is as follows

This means that the withdrawFromEscrow function returned true, indicating that the item token ID 15 was in escrow and successfully withdrawn, which contradicts the expectation that it should not be in escrow.

Tools Used

Foundry

Recommendations

  • Ensure that only the original depositor can withdraw the token from escrow.

  • Add a check to verify that the caller is the original depositor.

Updates

Lead Judging Commences

n0kto Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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