Summary
An attacker could continuously bridge L1 NFTs to L2, causing the white_listed_list number of elements to grow excessively and potentially leading to an out-of-gas DoS attack when sequencer calling withdraw_auto_from_l1.
Vulnerability Details
When users bridge an NFT collection from L1 to L2 and the corresponding L2 collection does not exist, it will be created, added to the whitelist, and included in the white_listed_list mapping by calling _white_list_collection. During the process of adding it to the white_listed_list, the function will iterate through the elements of white_listed_list until it finds the last element.
https://github.com/Cyfrin/2024-07-ark-project/blob/main/apps/blockchain/starknet/src/bridge.cairo#L472-L478
fn ensure_erc721_deployment(ref self: ContractState, req: @Request) -> ContractAddress {
self.l1_to_l2_addresses.write(l1_req, l2_addr_from_deploy);
self.l2_to_l1_addresses.write(l2_addr_from_deploy, l1_req);
self
.emit(
CollectionDeployedFromL1 {
l1_addr: *req.collection_l1,
l2_addr: l2_addr_from_deploy,
name: req.name.clone(),
symbol: req.symbol.clone()
}
);
let (already_white_listed, _) = self.white_listed_list.read(l2_addr_from_deploy);
if already_white_listed != true {
>>> _white_list_collection(ref self, l2_addr_from_deploy, true);
self.emit(CollectionWhiteListUpdated {
collection: l2_addr_from_deploy,
enabled: true,
});
}
l2_addr_from_deploy
}
https://github.com/Cyfrin/2024-07-ark-project/blob/main/apps/blockchain/starknet/src/bridge.cairo#L491-L542
fn _white_list_collection(ref self: ContractState, collection: ContractAddress, enabled: bool) {
let no_value = starknet::contract_address_const::<0>();
let (current, _) = self.white_listed_list.read(collection);
if current != enabled {
let mut prev = self.white_listed_head.read();
if enabled {
self.white_listed_list.write(collection, (enabled, no_value));
if prev.is_zero() {
self.white_listed_head.write(collection);
return;
}
>>> loop {
let (_, next) = self.white_listed_list.read(prev);
if next.is_zero() {
break;
}
let (active, _) = self.white_listed_list.read(next);
if !active {
break;
}
prev = next;
};
self.white_listed_list.write(prev, (true, collection));
} else {
if prev == collection {
let (_, next) = self.white_listed_list.read(prev);
self.white_listed_list.write(collection, (false, no_value));
self.white_listed_head.write(next);
return;
}
loop {
let (active, next) = self.white_listed_list.read(prev);
if next.is_zero() {
break;
}
if !active {
break;
}
if next == collection {
let (_, target) = self.white_listed_list.read(collection);
self.white_listed_list.write(prev, (active, target));
break;
}
};
self.white_listed_list.write(collection, (false, no_value));
}
}
}
This means that if the whitelist functionality is disabled on L1, an attacker could repeatedly call depositTokens with newly created NFT collections, causing new collections to be continually created on the L2 side, leading to an undesired state.
https://github.com/Cyfrin/2024-07-ark-project/blob/main/apps/blockchain/ethereum/src/Bridge.sol#L78-L144
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);
}
This scenario could also eventually occur without any malicious intent if the whitelist is disabled and enough different collections are added on L2, leading to an out-of-gas situation and potentially bricking the withdraw_auto_from_l1 functionality.
Impact
the withdraw_auto_from_l1 will be DoSed.
Tools Used
Manual review
Recommendations
Avoid the loop to find last element when adding the newly created collections to white_listed_list within with withdraw_auto_from_l1 to avoid this undesired state.
