When retrieving ERC721 collection metadata values (name, symbol, token_uri), we are retrieving their returned data to be ByteArray
. Since these functions will return string values we need to understand what is the data type of string value in Cairo.
Cairo is a low-level programming language, it does not support strings, and strings are represented using either felt252
or ByteArray
.
If the string length is smaller than 31
length it can be represented in only one felt252
.
First we need to understand why there are two types for retrieving strings.
The original type was normal felt252
, but on March 2024 Starknet introduced ByteArray
type.
This means that all NFT collections (ERC721 tokens) are created from the launching of the Starknet blockchain till the type of supporting ByteArray having an interface that returns the name
, symbol
, and token_uri
as felt252
.
OpenZepeplin starknet contract versions from the beginning to v0.9.0
use felt252
for their ERC721 tokens.
OZ::0.9.0::erc721.cairo#L219-L221
And from v0.10.0
to v0.15.0
they use ByteArray
. The protocol uses v0.11.0, which assigns the returned value to ByteArray
.
OZ::0.11.0::erc721.cairo#L221-L223
The Starknet launched November 2021
or February 2022
, and ByteArray
was supported on March 2024. so all ERC721 tokens created in these 2 years have an interface that returns felt252
for name
, symbol
, and token_uri
.
The protocol is not handling such a case and deals with NFTs on layer2 as they return ByteArray
when calling name
, symbol
, which is totally incorrect, as the majority of NFTs were created before supporting ByteArray
so dealing with the returned value when calling <collection>.name()
will only work for ERC721 tokens that was created after supporting ByteArray
, and use it.
When making calls there are two types of calls in starknet:
low-level call: similar to .call
in solidity.
High-level calls: interface calls.
In solidity the returned data is in Bytes whatever its type was, the same is in Starknet when doing low-level call the returned type is Span
We are handling retrieving token_uri correctly, where we are doing an internal call when getting them, which result in a return value as Span<felt252>
collection_manager.cairo#L107-L123
Since the returned value is felt252, we will convert it into ByteArray
without problems and there is a custom implementation for try_into
than handles converting felt252 into ByteArray correctly in byte_array_extra
.
The issue is that when getting token name
and symbol we are retrieving them with High-level call.
collection_manager.cairo#L69-L70
The interface we are defining ERC721Metadata
declares string values as ByteArray
we explained why uris
will get handled correctly, and for base_uri
it is written manually, but for name()
and symbol()
we are calling them directly and expect the returned value to be of type ByteArray
.
erc721
is the collection address we are going to Bridge the token from it from L2
to L1
, and since not all NFT collections on L2
, but actually the majority of NFT collections on L2
return felt252
, making erc721.name()
will result in an exception because of incorrect assigning. the execution will stop even we have an Option
return value because this is an execution error that occurred before reaching the end of the function.
Doing a POC for such an issue is complicated, but to make it achievable we separated it into two parts:
In the Starknet development environment make another erc721_collection
that returns name()
, symbol()
, token_uri()
as strings. Achieving this needs a lot of modifications to erc721_collection.cairo
file so we made another file erc721_collection_2.cairo
and make it as an NFT collection that returns felt252
.
Add the following test script in the last of apps/blockchain/starknet/src/tests/bridge_t.cairo
.
Run the following command.
Output
NOTE: To not make the report too large, we mentioned in point 1
what we need in order for the POC to work, all you have to do is to deploy an erc721_collection
that returns felt252
for name()
, and we preferred to leave this to you as mention how to make this needs a lot of modifications. And for the POC, you need to make sure that you are deploying the correct address, our POC works when there are two contracts erc721_collection
and erc721_collection_2
and the 2
version is the one that returns felt252
, you need to import these contract and make sure to add them in order to get them in artificats and not receive path errors. And ofc having the sponsor with you when setting it up is the best
The majourity of NFTs on L2
, which returns felt252
when calling name()
or symbol()
can't get briged.
Manual review + Starknet Foundry
reterive erc721.name()
and erc721.symbol()
using low-level to get the result as Span<felt252>
then convert them into ByteArray
.
Impact: Medium/High, no token loss but protocol do not work with a lot of ERC721 collections. Likelyhood: Very High, old collections created before the 08 Mar 2024 will not work, and according to those statistics, it is the huge majority of them: <https://dune.com/starknet_foundation/starknet-activity>
Impact: Medium/High, no token loss but protocol do not work with a lot of ERC721 collections. Likelyhood: Very High, old collections created before the 08 Mar 2024 will not work, and according to those statistics, it is the huge majority of them: <https://dune.com/starknet_foundation/starknet-activity>
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.