ArkProject: NFT Bridge
ArkProject
NFTBridge
60,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

L2-native collections cannot be properly bridged to L1

n4nika

Summary

If a user tries to bridge a L2-native NFT to L1 and it does not use a baseURI on L1, the NFT's metadata is not reflected on L1 when minting causing the NFT's metadata to be missing/lost.

Vulnerability Details

Currently when bridgeing from L1 to L2 there are two options, either the NFT is minted on L2 or it gets transferred from the bridge (if it has already been bridged L2->L1 before).
Now if the NFT gets minted and it has a token_uri associated with it, that data gets properly transmitted from L1 to L2, causing the NFT to be minted with mint_from_bridge_uri, ensuring that the NFT's uri gets preserved.
This is important as the uri contains information about a specific NFT like for example the image associated with it.

The problem is, that there is no such mechanism when bridging from L2 to L1. If an NFT gets minted by the bridge on L1, it gets minted with the function mintFromBridge which does not set a uri.

function withdrawTokens(
uint256[] calldata request
)
external
payable
returns (address)
{
// [...]
​
for (uint256 i = 0; i < req.tokenIds.length; i++) {
uint256 id = req.tokenIds[i];
​
bool wasEscrowed = _withdrawFromEscrow(ctype, collectionL1, req.ownerL1, id);
​
if (!wasEscrowed) {
// TODO: perhaps, implement the same interface for ERC721 and ERC1155
// As we only want to deal with ERC1155 token with value = 1.
// Also, check what to do with URIs. If the URI storage is supported
// or not for ERC721. If supported, we may need to mint with an URI.
IERC721Bridgeable(collectionL1).mintFromBridge(req.ownerL1, id);
}
}
// [...]
}

Now there is a TODO vaguely pointing out that this might be needed in the future, but it is a definite issue right now.

Impact

This causes NFTs bridged from L2 to L1 to be minted without metadata-uri. Since the metadata contains crucial information about an NFT, this is a major problem for users.

Proof of Concept

Local testnet setup needed for PoC:

Full testnet setup for PoC purposes

The following script adds all needed files for my PoC execution on a local testnet.
NOTE: this only needs to be executed ONCE for all my testnet-related PoCs to work! Also please only execute fullSetup.sh from the contest-directory apps/blockchain/!

How to use

  • Go to apps/blockchain/

  • Go to ethereum and execute forge test once

  • Go to ../starknet and execute snforge test once

  • Go back to ../ (should be back in apps/blockchain/)

  • Create file fullSetup.sh and add the script to it

  • Execute with sh fullSetup.sh

Launching testnet and deploying basic contracts

  • Open a new terminal and execute anvil here you will see all the logs of the L1

  • Open a new terminal, go to apps/blockchain/starknet/ and execute katana --messaging ./data/anvil.messaging.json --seed 0, here you will see all the logs of L2

  • Go to the original terminal (apps/blockchain/)

  • From here on, please follow the instructions of the specific PoC provided after this

What it does

It creates the following files needed to execute individual PoCs:

  • General files

    • foundry.toml

    • .env

  • Scripts (they get made executable with chmod +x <SCRIPTS>):

    • deploy.sh

    • adjustL1Bridge.sh

    • setMappingL1.sh

    • setMappingL2.sh

    • deployNFTonL1.sh

    • deployNFTonL2.sh

    • enableBridgeL1.sh

    • enableBridgeL2.sh

    • deployMetadataNFTonL1.sh

    • sourceAllMetadata.sh

  • Solidity files:

    • ./ethereum/src/token/ERC721BridgeableMetadata.sol

    • ./ethereum/src/token/IERC721BridgeableMetadata.sol

    • ./ethereum/script/ERC721Metadata.s.sol

Additionally it creates a ./ethereum/logs dir and creates the files ./ethereum/logs/local_messaging_deploy.json and ./ethereum/logs/starklane_deploy.json, which was needed for me to get script execution working.

Script

cat <<'EOF' > ./.env
STARKNET_RPC=http://0.0.0.0:5050
​
# Deployer
STARKNET_DEPLOYER_ACCOUNT=./starknet/data/katana_deployer_account.json
STARKNET_DEPLOYER_PRIVATE_KEY=0x6bf3604bcb41fed6c42bcca5436eeb65083a982ff65db0dc123f65358008b51
STARKNET_DEPLOYER_ADDR=0x56c155b624fdf6bfc94f7b37cf1dbebb5e186ef2e4ab2762367cd07c8f892a1
​
# Bridge admin
STARKNET_ADMIN_ACCOUNT=./starknet/data/katana_admin_account.json
STARKNET_ADMIN_PRIVATE_KEY=0x283d1e73776cd4ac1ac5f0b879f561bded25eceb2cc589c674af0cec41df441
STARKNET_ADMIN_ADDR=0x66efb28ac62686966ae85095ff3a772e014e7fbf56d4c5f6fac5606d4dde23a
​
# Owner of ERC721 collection
STARKNET_OWNER_ACCOUNT=./starknet/data/katana_owner_account.json
STARKNET_OWNER_PRIVATE_KEY=0x1c9053c053edf324aec366a34c6901b1095b07af69495bffec7d7fe21effb1b
STARKNET_OWNER_ADDR=0x6b86e40118f29ebe393a75469b4d926c7a44c2e2681b6d319520b7c1156d114
​
# User
STARKNET_USER_ACCOUNT=./starknet/data/katana_account.json
STARKNET_USER_PRIVATE_KEY=0x1800000000300000180000000000030000000000003006001800006600
STARKNET_USER_ACCOUNT_ADDR=0x6162896d1d7ab204c7ccac6dd5f8e9e7c25ecd5ae4fcb4ad32e57786bb46e03
​
ETH_ACCOUNT=0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
​
# General config.
ETH_RPC_URL=http://127.0.0.1:8545
LOCAL_LOGS=logs/
ETHERSCAN_API_KEY=fake_key
​
# Account related variables (EOA account).
ACCOUNT_PRIVATE_KEY=0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80
ACCOUNT_ADDRESS=0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
​
ETHEREUM_USER_ADDR=0x70997970C51812dc3A010C7d01b50e0d17dc79C8
ETHEREUM_USER_PRIVATE_KEY=0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d
​
# Starklane L1 variables.
STARKNET_CORE_L1_ADDRESS=0x0000000000000000000000000000000000000000
STARKLANE_L1_PROXY_ADDRESS=0x0000000000000000000000000000000000000000
# STARKLANE_L1_PROXY_ADDRESS=0x9fe46736679d2d9a65f0992f2272de9f3c7fa6e0
​
# Starklane L2 variables.
STARKLANE_L2_ADDRESS=0x075af152a7ad8c32726093fdf9820de103854d15a8ef4fae11d2739671ff1f9e
STARKLANE_L2_SELECTOR=0x005d25c8970b257e237fddc686e4a2711310774d2277107e1ebaf3e0a68dd037
OWNER_L2_ADDRESS=0x1
​
alias cast_admin_send="cast send --private-key ${ACCOUNT_PRIVATE_KEY}"
alias cast_user_send="cast send --private-key ${ETHEREUM_USER_PRIVATE_KEY}"
# starkli alias
alias starkli_deployer="STARKNET_RPC=${STARKNET_RPC} STARKNET_ACCOUNT=${STARKNET_DEPLOYER_ACCOUNT} STARKNET_PRIVATE_KEY=${STARKNET_DEPLOYER_PRIVATE_KEY} starkli"
alias starkli_admin="STARKNET_RPC=${STARKNET_RPC} STARKNET_ACCOUNT=${STARKNET_ADMIN_ACCOUNT} STARKNET_PRIVATE_KEY=${STARKNET_ADMIN_PRIVATE_KEY} starkli"
alias starkli_owner="STARKNET_RPC=${STARKNET_RPC} STARKNET_ACCOUNT=${STARKNET_OWNER_ACCOUNT} STARKNET_PRIVATE_KEY=${STARKNET_OWNER_PRIVATE_KEY} starkli"
alias starkli_user="STARKNET_RPC=${STARKNET_RPC} STARKNET_ACCOUNT=${STARKNET_USER_ACCOUNT} STARKNET_PRIVATE_KEY=${STARKNET_USER_PRIVATE_KEY} starkli"
EOF
​
​
cat <<'EOF' > ./foundry.toml
[profile.default]
src = "src"
out = "out"
libs = ["lib"]
test = "test"
cache_path = "cache"
root = "./ethereum/"
script = "script"
broadcast = "broadcast"
gas_limit = 999999999
fs_permissions = [{ access = "read-write", path = "logs"}]
​
remappings = [
"ds-test/=lib/forge-std/lib/ds-test/src/",
"erc4626-tests/=lib/openzeppelin-contracts/lib/erc4626-tests/",
"forge-std/=lib/forge-std/src/",
"openzeppelin-contracts/=lib/openzeppelin-contracts/",
"openzeppelin/=lib/openzeppelin-contracts/contracts/",
"starknet/=lib/starknet/",
]
​
# See more config options https://github.com/foundry-rs/foundry/tree/master/config
​
[rpc_endpoints]
goerli = "${GOERLI_RPC_URL}"
anvil = "http://127.0.0.1:8545"
​
​
[etherscan]
goerli = { key = "${ETHERSCAN_API_KEY}" }
anvil = { key = "FAKE KEY", chainId=31337}
EOF
​
​
cat <<'EOF' > ./deploy.sh
source ./.env
​
forge script --broadcast --rpc-url anvil ./ethereum/script/LocalMessaging.s.sol:LocalMessaging
​
STARKNET_CORE_L1_ADDRESS=$(cat ./ethereum/logs/local_messaging_deploy.json | jq '.data.sncore_address' | sed 's|"||g') forge script --broadcast --rpc-url anvil ./ethereum/script/Starklane.s.sol:Deploy
BRIDGE_L1_ADDR=$(cat ./ethereum/logs/starklane_deploy.json | jq '.data.proxy_address' | sed 's|"||g')
​
ERC721_BRIDGEABLE_CLASSHASH=$(starkli_deployer declare --compiler-version 2.6.2 --watch ./starknet/target/dev/starklane_erc721_bridgeable.contract_class.json | tail -n 1)
​
echo "ERC721_BRIDGEABLE_CLASSHASH: $ERC721_BRIDGEABLE_CLASSHASH"
​
BRIDGE_CLASSHASH=$(starkli_deployer declare --compiler-version 2.6.2 --watch ./starknet/target/dev/starklane_bridge.contract_class.json | tail -n 1)
​
echo "BRIDGE_CLASSHASH: $BRIDGE_CLASSHASH"
​
echo "STARKNET_ADMIN_ADDR: $STARKNET_ADMIN_ADDR"
​
echo "BRIDGE_L1_ADDR: $BRIDGE_L1_ADDR"
​
BRIDGE_L2_ADDR=$(starkli_deployer deploy --salt $(date +%s%N) --watch ${BRIDGE_CLASSHASH} ${STARKNET_ADMIN_ADDR} ${BRIDGE_L1_ADDR} ${ERC721_BRIDGEABLE_CLASSHASH} | tail -n 1)
​
echo "BRIDGE_L2_ADDR: $BRIDGE_L2_ADDR"
EOF
​
​
cat <<'EOF' > ./enableBridgeL1.sh
cast_admin_send ${BRIDGE_L1_ADDR} "enableBridge(bool)" true
EOF
​
​
cat <<'EOF' > ./enableBridgeL2.sh
starkli_admin invoke --watch ${BRIDGE_L2_ADDR} enable 1
EOF
​
​
cat <<'EOF' > ./adjustL1Bridge.sh
cast_admin_send ${BRIDGE_L1_ADDR} "setStarklaneL2Address(uint256)" ${BRIDGE_L2_ADDR}
​
STARKLANE_L2_SELECTOR=0x3593216f3a8b22f4cf375e5486e3d13bfde9d0f26976d20ac6f653c73f7e507
​
cast_admin_send ${BRIDGE_L1_ADDR} "setStarklaneL2Selector(uint256)" ${STARKLANE_L2_SELECTOR}
EOF
​
​
cat <<'EOF' > ./deployNFTonL1.sh
ERC721_PROXY_ADDRESS=0x0000000000000000000000000000000000000000 forge script --broadcast --rpc-url anvil ./ethereum/script/ERC721.s.sol:Deploy
ERC721_L1_ADDR=$(cat ./ethereum/logs/erc721_deploy.json | jq '.data.proxy_address' | sed 's|"||g')
EOF
​
​
cat <<'EOF' > ./deployMetadataNFTonL1.sh
ERC721_PROXY_ADDRESS=0x0000000000000000000000000000000000000000 forge script --broadcast --rpc-url anvil ./ethereum/script/ERC721Metadata.s.sol:Deploy
ERC721_L1_ADDR=$(cat ./ethereum/logs/erc721_deploy.json | jq '.data.proxy_address' | sed 's|"||g')
EOF
​
​
cat <<'EOF' > ./deployNFTonL2.sh
ERC721_L2_ADDR=$(starkli_deployer deploy --salt $(date +%s%N) --watch ${ERC721_BRIDGEABLE_CLASSHASH} 0x0 0x636f6c6c656374696f6e5f74657374 0xf 0x0 0x4354455354 0x5 0x0 0x555249 0x3 ${BRIDGE_L2_ADDR} ${STARKNET_OWNER_ADDR} | tail -n 1)
EOF
​
​
cat <<'EOF' > ./setMappingL1.sh
cast_admin_send ${BRIDGE_L1_ADDR} "setL1L2CollectionMapping(address,uint256,bool)" ${ERC721_L1_ADDR} ${ERC721_L2_ADDR} true
EOF
​
​
cat <<'EOF' > ./setMappingL2.sh
starkli_admin invoke --watch ${BRIDGE_L2_ADDR} set_l1_l2_collection_mapping ${ERC721_L1_ADDR} ${ERC721_L2_ADDR}
EOF
​
​
cat <<'EOF' > ./sourceAllMetadata.sh
source ./deploy.sh
source ./adjustL1Bridge.sh
​
source ./deployMetadataNFTonL1.sh
source ./deployNFTonL2.sh
​
source ./setMappingL1.sh
source ./setMappingL2.sh
​
source ./enableBridgeL1.sh
source ./enableBridgeL2.sh
EOF
​
​
chmod +x deploy.sh adjustL1Bridge.sh setMappingL1.sh setMappingL2.sh deployNFTonL1.sh deployNFTonL2.sh enableBridgeL1.sh enableBridgeL2.sh deployMetadataNFTonL1.sh sourceAllMetadata.sh
​
mkdir ethereum/logs
touch ethereum/logs/local_messaging_deploy.json
touch ethereum/logs/starklane_deploy.json
​
​
cat <<'EOF' > ./ethereum/src/token/ERC721BridgeableMetadata.sol
// SPDX-License-Identifier: MIT
​
pragma solidity ^0.8.0;
​
import "openzeppelin-contracts/contracts/token/ERC721/ERC721.sol";
​
import "./IERC721Bridgeable.sol";
import "../UUPSProxied.sol";
​
contract ERC721BridgeableMetadata is ERC721, UUPSOwnableProxied, IERC721Bridgeable {
​
string private _name;
​
string private _symbol;
​
string private __baseURI;
​
address private _bridge;
​
mapping(uint256 => string) private _tokenURIs;
​
constructor()
ERC721("", "")
{ }
​
function initialize(
bytes calldata data
)
public
onlyInit
{
(string memory n, string memory s) = abi.decode(data, (string, string));
​
_name = n;
_symbol = s;
​
_transferOwnership(_msgSender());
}
​
function mintFromBridge(address to, uint256 id)
public
{
​
assert(msg.sender == address(_bridge));
​
_mint(to, id);
}
​
function burnFromBridge(uint256 id)
public
{
assert(msg.sender == address(_bridge));
​
_burn(id);
}
function setBridge(address bridge) public onlyOwner {
assert(bridge != address(0));
_bridge = bridge;
}
​
function _baseURI()
internal
view
override
returns (string memory) {
return __baseURI;
}
​
function _setTokenURI(uint256 tokenId, string calldata _tokenURI) internal {
_requireMinted(tokenId);
_tokenURIs[tokenId] = _tokenURI;
}
​
function _setBaseURI(string memory baseURI_) internal {
__baseURI = baseURI_;
}
​
function baseURI() public view returns (string memory) {
return __baseURI;
}
​
function setBaseURI(string memory baseURI_) public onlyOwner {
_setBaseURI(baseURI_);
}
​
function setTokenURI(uint256 tokenId, string calldata _tokenURI) public onlyOwner {
_setTokenURI(tokenId, _tokenURI);
}
​
function mintWithTokenURI(address to, uint256 tokenId, string calldata _tokenURI) public onlyOwner returns (bool) {
_mint(to, tokenId);
​
_setTokenURI(tokenId, _tokenURI);
​
return true;
}
​
function mint(address to, uint256 tokenId) public onlyOwner {
_mint(to, tokenId);
}
​
function name()
public
view
override
returns (string memory)
{
return _name;
}
​
function symbol()
public
view
override
returns (string memory)
{
return _symbol;
}
}
EOF
​
​
cat <<'EOF' > ./ethereum/src/token/IERC721BridgeableMetadata.sol
pragma solidity ^0.8.0;
​
interface IERC721BridgeableMetadata {
​
function initialize(bytes calldata data) external;
​
function setBaseURI(string calldata baseURI_) external;
​
function setTokenURI(uint256 tokenId, string calldata _tokenURI) external;
​
function mintWithTokenURI(address to, uint256 tokenId, string calldata _tokenURI) external;
​
function tokenURI(uint256 tokenId) external view returns (string memory);
}
EOF
​
​
cat <<'EOF' > ./ethereum/script/ERC721Metadata.s.sol
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import "forge-std/Script.sol";
​
import "./Utils.sol";
import "src/token/ERC721BridgeableMetadata.sol";
import "openzeppelin-contracts/contracts/proxy/ERC1967/ERC1967Proxy.sol";
​
contract Deploy is Script {
​
function setUp() public {
}
​
function run() public {
Config memory config = Utils.loadConfig();
vm.startBroadcast(config.deployerPrivateKey);
​
address impl = address(new ERC721BridgeableMetadata());
bytes memory dataInit = abi.encodeWithSelector(
ERC721BridgeableMetadata.initialize.selector,
abi.encode(
"metadata_token",
"MDT"
)
);
​
address proxyAddress = vm.envAddress("ERC721_PROXY_ADDRESS");
​
if (proxyAddress == address(0x0)) {
proxyAddress = address(new ERC1967Proxy(impl, dataInit));
} else {
ERC721BridgeableMetadata(payable(proxyAddress)).upgradeToAndCall(impl, dataInit);
}
​
vm.stopBroadcast();
​
string memory json = "erc721_deploy";
vm.serializeString(json, "proxy_address", vm.toString(proxyAddress));
vm.serializeString(json, "impl_address", vm.toString(impl));
Utils.writeJson(json, "erc721_deploy.json");
}
}
EOF

Bug-specific PoC instructions

At this point you should have three running terminals. One executing anvil, one executing katana ... and a third one allowing us to interact with the testnet.

To show that bridging an NFT with a URI from L2 to L1 causes the URI to be lost on L1, please now do the following:

Add the following content to a new file poc-LostMetadata.sh:

source ./sourceAllMetadata.sh
cast_admin_send ${ERC721_L1_ADDR} "setBridge(address)" $BRIDGE_L1_ADDR
starkli_owner invoke --watch ${ERC721_L2_ADDR} mint_range ${STARKNET_USER_ACCOUNT_ADDR} u256:0 u256:10
starkli_user invoke --watch ${ERC721_L2_ADDR} approve ${BRIDGE_L2_ADDR} u256:1

Then execute it with source ./poc-LostMetadata.sh

This does the following:

  • set up the bridge (L1 and L2)

  • deploy NFTs on L1 and L2

  • set up L1 <-> L2 mappings on both chains

  • mints NFTs to user on L2

  • approves NFT 1 for bridging

Now that everything is set up, we can finally show the bug by doing the following:

  • We get the uri of NFT 1 on L2 by executing: starkli_user call ${ERC721_L2_ADDR} token_uri u256:1 this shows that the URI of our NFT is URI1 (encoded)

  • We bridge NFT 1 to L1: starkli_user invoke --watch ${BRIDGE_L2_ADDR} deposit_tokens 10 ${ERC721_L2_ADDR} ${ETHEREUM_USER_ADDR} 1 u256:1 0 0

  • In order to get the message payload we need to withdraw on L1, do the following:

    • Get the transaction hash which was printed in the console. For example: Transaction 0x030aa979a12028a2125ea88eda5ad98a0d3decc991d4d55ef28e984689da3000 confirmed -> 0x030aa979a12028a2125ea88eda5ad98a0d3decc991d4d55ef28e984689da3000

    • Execute starkli_user tx-receipt 0x030aa979a12028a2125ea88eda5ad98a0d3decc991d4d55ef28e984689da3000 | jq '.messages_sent[0].payload' with your transaction hash

    • Take the result and format it into the following format: "[0x1,0x2,0x3,0x4,0x5,...]"

    • It should look similar to this: [0x101,0x12da98742ed634a4e595621efa974a81,0x384bca09b8087bb0952c044578fae999,0xa513e6e4b8f2a923d98304ec87f64353c4d5c853,0x1598c82cc5694894da10861fa3a509b62d91b4ab68740169d5f2c9d1c23c3ab,0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266,0x6162896d1d7ab204c7ccac6dd5f8e9e7c25ecd5ae4fcb4ad32e57786bb46e03,0x0,0x636f6c6c656374696f6e5f74657374,0xf,0x0,0x4354455354,0x5,0x0,0x0,0x0,0x1,0x1e,0x0,0x0,0x1,0x0,0x5552493330,0x5,0x0]

  • Now to withdraw on L2, execute the following: cast_user1_send ${BRIDGE_L1_ADDR} "withdrawTokens(uint256[])" "<YOUR_REQ>"

  • We now check the uri on L1 by calling: cast call --private-key ${ETHEREUM_USER_PRIVATE_KEY} ${ERC721_L1_ADDR} "tokenURI(uint256)" 1

  • This will return something along the lines of 0x00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000000 showing that the NFT's uri is not set on L1!

Tools Used

Manual review

Recommended Mitigation

In order to fix this, one could either implement the mintFromBridgeUri function in ERC721Bridgeable and call that if the request contains token uris, or add some other functionality setting the NFT's uri in that case.

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

invalid-NFT-minted-without-baseURI-URIs-or-bridge-with-no-metadata

URI is not lost on the origin chain and it can be modified with `ERC721UriImpl`. As explained in the TODO  below, that’s a design choice and it will be implemented as a future feature. https://github.com/Cyfrin/2024-07-ark-project/blob/main/apps/blockchain/ethereum/src/Bridge.sol#L206 `ERC721Bridgable` is out of scope.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.