Summary
Unauthorized withdrawal vulnerability in withdrawTokens function.
Vulnerability Details
The withdrawTokens function and _withdrawFromEscrow function didn't implement any checks to verify that the caller has the right to withdraw the specified tokens. It relies solely on the provided request data without authenticating the requester's ownership or permission.
function withdrawTokens(
uint256[] calldata request
)
external
payable
returns (address)
{
if (!_enabled) {
revert BridgeNotEnabledError();
}
uint256 header = request[0];
if (Protocol.canUseWithdrawAuto(header)) {
revert NotSupportedYetError();
} else {
_consumeMessageStarknet(_starknetCoreAddress, _starklaneL2Address, request);
}
Request memory req = Protocol.requestDeserialize(request, 0);
address collectionL1 = _verifyRequestAddresses(req.collectionL1, req.collectionL2);
CollectionType ctype = Protocol.collectionTypeFromHeader(header);
if (collectionL1 == address(0x0)) {
if (ctype == CollectionType.ERC721) {
collectionL1 = _deployERC721Bridgeable(
req.name,
req.symbol,
req.collectionL2,
req.hash
);
_whiteListCollection(collectionL1, true);
} else {
revert NotSupportedYetError();
}
}
for (uint256 i = 0; i < req.tokenIds.length; i++) {
uint256 id = req.tokenIds[i];
bool wasEscrowed = _withdrawFromEscrow(ctype, collectionL1, req.ownerL1, id);
if (!wasEscrowed) {
IERC721Bridgeable(collectionL1).mintFromBridge(req.ownerL1, id);
}
}
emit WithdrawRequestCompleted(req.hash, block.timestamp, request);
return collectionL1;
}
https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/Bridge.sol#L153C3-L214C29
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;
}
https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/Escrow.sol#L63C1-L90C1
Assume Alice has deposited an ERC721 token with ID 123 from collection 0x1234. An attacker Bob could call:
This call would allow Bob to withdraw or have minted to his address the token that Alice deposited.
Impact
An attacker could craft a withdrawal request for tokens they don't own, withdrawing escrowed tokens belonging to other users. And minting new tokens to an address of their choice.
Tools Used
Manual review
Recommendations
Implement a mapping of depositors to their deposited tokens. And add a check in the withdrawTokens function to ensure the caller is the original depositor:
