Summary
A malicious actor could repeatedly replay the same transaction to transfer tokens multiple times. Since there is no mechanism to mark a request as processed, this can lead to multiple token transactions, effectively stealing tokens.
Vulnerability Details
The depositTokens function does not store the hash of the request, leaving it vulnerable to reply in the same transaction multiple times to steal tokens
function depositTokens(
uint256 salt,
address collectionL1,
snaddress ownerL2,
uint256[] calldata ids,
bool useAutoBurn
)
external
payable
{
if (!Cairo.isFelt252(snaddress.unwrap(ownerL2))) {
revert CairoWrapError();
}
if (!_enabled) {
revert BridgeNotEnabledError();
}
CollectionType ctype = TokenUtil.detectInterface(collectionL1);
if (ctype == CollectionType.ERC1155) {
revert NotSupportedYetError();
}
if (!_isWhiteListed(collectionL1)) {
revert NotWhiteListedError();
}
Request memory req;
req.header = Protocol.requestHeaderV1(ctype, useAutoBurn, false);
req.hash = Protocol.requestHash(salt, collectionL1, ownerL2, ids);
req.collectionL1 = collectionL1;
req.collectionL2 = _l1ToL2Addresses[collectionL1];
req.ownerL1 = msg.sender;
req.ownerL2 = ownerL2;
if (ctype == CollectionType.ERC721) {
(req.name, req.symbol, req.uri, req.tokenURIs) = TokenUtil.erc721Metadata(
collectionL1,
ids
);
} else {
(req.uri) = TokenUtil.erc1155Metadata(collectionL1);
}
_depositIntoEscrow(ctype, collectionL1, ids);
req.tokenIds = ids;
uint256[] memory payload = Protocol.requestSerialize(req);
if (payload.length >= MAX_PAYLOAD_LENGTH) {
revert TooManyTokensError();
}
IStarknetMessaging(_starknetCoreAddress).sendMessageToL2{value: msg.value}(
snaddress.unwrap(_starklaneL2Address),
felt252.unwrap(_starklaneL2Selector),
payload
);
emit DepositRequestInitiated(req.hash, block.timestamp, payload);
}
PoC
Step 1: Assume an attacker initiates a valid token deposit using the depositTokens function
Step 2: The attacker captures the transaction data(salt, collectionL1, ownerL2, ids, and useAutoBurn).
Step3: The attacker replays the captured transaction multiple times by sending the same data to the depositTokens function.
Step 4: Since the contract does not store the hash and check for duplicates, it processes the replayed transaction each time, leading to unauthorised multiple deposits and potential loss of tokens
Code snippet
function depositTokens(
uint256 salt,
address collectionL1,
snaddress ownerL2,
uint256[] calldata ids,
bool useAutoBurn
)
external
payable
{
if (!Cairo.isFelt252(snaddress.unwrap(ownerL2))) {
revert CairoWrapError();
}
if (!_enabled) {
revert BridgeNotEnabledError();
}
CollectionType ctype = TokenUtil.detectInterface(collectionL1);
if (ctype == CollectionType.ERC1155) {
revert NotSupportedYetError();
}
if (!_isWhiteListed(collectionL1)) {
revert NotWhiteListedError();
}
Request memory req;
req.header = Protocol.requestHeaderV1(ctype, useAutoBurn, false);
req.hash = Protocol.requestHash(salt, collectionL1, ownerL2, ids);
req.collectionL1 = collectionL1;
req.collectionL2 = _l1ToL2Addresses[collectionL1];
req.ownerL1 = msg.sender;
req.ownerL2 = ownerL2;
if (ctype == CollectionType.ERC721) {
(req.name, req.symbol, req.uri, req.tokenURIs) = TokenUtil.erc721Metadata(
collectionL1,
ids
);
} else {
(req.uri) = TokenUtil.erc1155Metadata(collectionL1);
}
_depositIntoEscrow(ctype, collectionL1, ids);
req.tokenIds = ids;
uint256[] memory payload = Protocol.requestSerialize(req);
if (payload.length >= MAX_PAYLOAD_LENGTH) {
revert TooManyTokensError();
}
IStarknetMessaging(_starknetCoreAddress).sendMessageToL2{value: msg.value}(
snaddress.unwrap(_starklaneL2Address),
felt252.unwrap(_starklaneL2Selector),
payload
);
emit DepositRequestInitiated(req.hash, block.timestamp, payload);
}
Impact
Loss could be the total value of all replayed tokens. If 1,000 tokens are worth $100 each, and the transaction is replayed 5 times, the loss could be $500,000
Tools Used
Manual review
Recommendations
Implement replay protection by storing each request hash and checking it before processing the transaction
mapping(bytes32 => bool)public
processedRequests;
function depositTokens(
uint256 salt,
address collectionL1,
snaddress ownerL2,
uint256[] calldata ids,
bool useAutoBurn
)
external
payable
{
bytes32 reqHash =
Protocol.requestHash(salt, collectionL1, ownerL2, ids);
require(!
processedRequests[reqHash], "Request already processed");
processedRequests[reqHash] = true;
}
