Summary

This report outlines the presence of potential reentrancy vulnerabilities in the StarklaneEscrow._withdrawFromEscrow() and Starklane.withdrawTokens() functions within the Starklane smart contract. These vulnerabilities arise due to external calls being made before state changes, potentially allowing attackers to exploit these functions to manipulate escrowed assets.

Vulnerability Details

StarklaneEscrow._withdrawFromEscrow(CollectionType, address, address, uint256)

Location: src/Escrow.sol lines 57-79

• External Calls:

  • IERC721(collection).safeTransferFrom(from, to, id) (line 68)

  • IERC1155(collection).safeTransferFrom(from, to, id, 1, "") (line 73)

• State Variables Written After Calls:

  • _escrow[collection][id] = address(0x0) (line 76)

Location: src/Bridge.sol lines 139-191

• External Calls:

  • _consumeMessageStarknet(_starknetCoreAddress, _starklaneL2Address, request) (line 154)

    • msgHash = starknetCore.consumeMessageFromL2(snaddress.unwrap(fromL2Address), request) (line 89 in Messaging.sol)

  • collectionL1 = _deployERC721Bridgeable(req.name, req.symbol, req.collectionL2, req.hash) (line 165)

    • proxy = Deployer.deployERC721Bridgeable(name, symbol) (line 50 in CollectionManager.sol)

    • address(new ERC1967Proxy(impl, dataInit)) (line 37 in Deployer.sol)

  • wasEscrowed = _withdrawFromEscrow(ctype, collectionL1, req.ownerL1, id) (line 176)

    • IERC721(collection).safeTransferFrom(from, to, id) (line 68 in Escrow.sol)

    • IERC1155(collection).safeTransferFrom(from, to, id, 1, "") (line 73 in Escrow.sol)

  • IERC721Bridgeable(collectionL1).mintFromBridge(req.ownerL1, id) (line 184)

• State Variables Written After Calls:

  • wasEscrowed = _withdrawFromEscrow(ctype, collectionL1, req.ownerL1, id) (line 176)

    • _escrow[collection][id] = address(0x0) (line 76 in Escrow.sol)

• Cross Function Reentrancy:

  • Functions that manipulate _escrow state variable:

    • _depositIntoEscrow(CollectionType, address, uint256[]) (lines 25-45 inEscrow.sol`)

    • _isEscrowed(address, uint256) (lines 89-91 in Escrow.sol)

    • _withdrawFromEscrow(CollectionType, address, address, uint256) (lines 57-79 in Escrow.sol)

Impact

If an attacker can exploit the reentrancy vulnerability:

• They may call external contracts in a way that reenters StarklaneEscrow._withdrawFromEscrow() and manipulatively alter the _escrow state variable before it's correctly updated.

• There is potential for unauthorized withdrawals, causing loss or incorrect handling of tokens.

• This could lead to significant financial loss and undermine the integrity of the Starklane platform.

Tools Used

Manual code review.

Recommendations

Reentrancy Guard:

• Introduce a reentrancy guard to prevent reentrancy attacks. The OpenZeppelin ReentrancyGuard contract is a common and reliable solution.

Check-Effects-Interactions Pattern:

Follow the "Check-Effects-Interactions" pattern to update state variables before making external calls. This can help ensure the contract's state is correct before any interactions with external contracts or addresses.