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

Loss of NFTs as the L2 side does currently not disallow requests with `use_withdraw_auto` set

n4nika

Summary

If users use the use_withdraw_auto feature when bridging from L2 to L1, their NFTs will be stuck in the L1 bridge.

Vulnerability Details

On L1, any requests using the use_withdraw_auto feature cannot be withdrawn as it has been disabled after the previous audit.

function withdrawTokens(
uint256[] calldata request
)
external
payable
returns (address)
{
if (!_enabled) {
revert BridgeNotEnabledError();
}
​
// Header is always the first uint256 of the serialized request.
uint256 header = request[0];
​
// Any error or permission fail in the message consumption will cause a revert.
// After message being consumed, it is considered legit and tokens can be withdrawn.
if (Protocol.canUseWithdrawAuto(header)) {
// 2024-03-19: disabled autoWithdraw after audit report
// _consumeMessageAutoWithdraw(_starklaneL2Address, request);
revert NotSupportedYetError();
} else {
_consumeMessageStarknet(_starknetCoreAddress, _starklaneL2Address, request);
}
​
// [...]
}

The problem is, that the L2 side of the bridge currently still allows deposit requests with that flag set to true.

fn deposit_tokens(
ref self: ContractState,
salt: felt252,
collection_l2: ContractAddress,
owner_l1: EthAddress,
token_ids: Span<u256>,
use_withdraw_auto: bool,
use_deposit_burn_auto: bool
) {
// [...]
​
let req = Request {
header: compute_request_header_v1(ctype, use_deposit_burn_auto, use_withdraw_auto), // <-----
hash: compute_request_hash(salt, collection_l2, owner_l1, token_ids),
collection_l1,
collection_l2,
owner_l1,
owner_l2: from,
name,
symbol,
base_uri,
ids: token_ids,
values: array![].span(),
uris,
new_owners: array![].span(),
};
​
// [...]
}

This means that a user can send a deposit with use_withdraw_auto=true which will be successful on L2 but any withdrawals on L1 will fail.

Impact

The impact of this is permanent locking of user NFTs in the L1 bridge in some scenarios.

Let's dive into why it is permanent:

So there are two scenarios:

  1. The bridged NFT comes originally from L2 and has not yet been minted on L1

  2. The bridged NFT origins on L1 and therefore is escrowed in the L1 bridge

In scenario 1), the NFT could be retrieved by an admin as it would probably be possible to mint the NFT on L1 as it was not yet minted.
In scenarion 2) however (which is the case for the Everai collection as it origins from L1) the NFT would be permanently locked in the bridge.
This is because as it was already minted it is not possible to mint it again. And since the only possibilities to get NFTs from the bridge other than minting are with withdrawTokens, which will revert, and cancelRequest, which can only be called for a pending L1 -> L2 bridge. Therefore not even an admin can retrieve the NFT.

Proof of Concept

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. 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.

Now to show this vulnerability, please do the following:

Add the following content to a new file poc-WithdrawAuto.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:20 u256:40
starkli_user invoke --watch ${ERC721_L2_ADDR} approve ${BRIDGE_L2_ADDR} u256:30

Then execute it with source ./poc-WithdrawAuto.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 30 for bridging

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

  • We then bridge the NFT with the use_withdraw_auto flag set to 1 which is allowed on L2: starkli_user invoke --watch ${BRIDGE_L2_ADDR} deposit_tokens $(date +%s%N) ${ERC721_L2_ADDR} ${ETH_ACCOUNT} 1 u256:30 1 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: [0x1000101,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>"

  • This will revert with the selector of NotSupportedYetError locking the NFT in the bridge

Tools Used

Manual review

Recommended Mitigation

I would recommend disallowing use_withdraw_auto withdrawals on L2 until they are enabled again on L1. Just pass false when building the request header, preventing this issue.

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-auto_withdrawn-L2-NFT-stuck

Impact: High, token will be stuck in L2 bridge. Likelyhood: Very low, option is available in L2 but has been disabled since March on L1, would be almost a user error.

Support

FAQs

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