Summary

The token_uri_from_contract_call function fails to handle Starknet's internal low-level call_contract_syscall thereby bypassing the intended fallback logic.

Vulnerability Details

The token_uri_from_contract_call function attempts to retrieve metadata (such as the token URI) for an ERC721 token on Starknet. The function uses call_contract_syscall to make internal calls to the token contract's token_uri and tokenURI functions.

The function uses match statements to handle the results of low-level call_contract_syscall.

It attempts to handle errors from call_contract_syscall, but due to the nature of internal calls in Starknet, these error cases will never actually be reached. Instead, if either call fails, the entire transaction will be reverted.

According to Starknet Documentation:

An internal call can’t return Err(_) as this is not handled by the sequencer and the Starknet OS.
If call_contract_syscall fails, this can’t be caught and will therefore result in the entire transaction being reverted.

Thus if a call_contract_syscall fails, the failure would lead to an immediate transaction revert, not just an error returned as a value. This is a critical point to consider in Starknet, as it implies that handling errors via match statements (Result::Ok or Result::Err) may not work as one would expect normally.

If the first call to token_uri succeeds, it returns the result.
If the first call fails, the entire transaction is reverted, and the second call is never attempted. The fallback logic to try tokenURI is effectively dead code.

The function will only succeed if the called contract has a token_uri function that doesn't revert.
Contracts that only implement tokenURI (capital URI) will always cause this function to fail, reverting the entire deposit transaction.
This creates a significant limitation on which NFT contracts can be successfully bridged.

Impact

The inability to catch and handle errors from call_contract_syscall means that the call to erc721_metadata will fail if token_uri_from_contract_call reverts.
This means that the entire deposit process will fail if the NFT contract doesn't have a working token_uri function, even if the contract is otherwise valid and the token exists. Valid NFTs might be unbridgeable due to this limitation.

Tools Used

Manual Review, StarkNet Documentation

Recommendations

If the interface of the called contract is available, then you can use a more straightforward syntax rather than using low-level call_contract_syscall