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

Insecure Handling of Caller Address in `deposit_tokens` Function Leads to Potential DOS

0xshoonya

Summary

The deposit_tokens function in the bridge.cairo contract fails to validate the caller's address obtained using starknet::get_caller_address(). In StarkNet's account abstraction model, this could lead to security vulnerabilities if the function is called directly without an account contract, resulting in a zero address for the caller.

Vulnerability Details

In StarkNet, the account abstraction model differs significantly from Ethereum. StarkNet does not use Externally Owned Accounts (EOAs); instead, all user interactions occur through smart contracts known as account contracts. These account contracts handle user authentication and interact with other contracts on behalf of the user.

The deposit_tokens function in bridge.cairo uses starknet::get_caller_address() without validating the returned address:

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 from = starknet::get_caller_address();
​
// ...
​
@> escrow_deposit_tokens(ref self, collection_l2, from, token_ids);
​
// ...
​
let mut buf: Array<felt252> = array![];
req.serialize(ref buf);
​
starknet::send_message_to_l1_syscall(
self.bridge_l1_address.read().into(),
buf.span(),
)
.unwrap_syscall();
// ...

The deposit_tokens function assumes it will always be called through an account contract, which is not guaranteed in StarkNet's model.

According to StarkNet's behavior, get_caller_address() could return 0 if the contract is called directly (i.e., not through an account contract).

It is possible to interact with contracts directly (e.g. through a StarkNet API or SDK). But from the perspective of the bridge.cairo contract, the caller's address will be zero.

This is different from calls made through an account contract, where get_caller_address() would return the account contract's address.

This zero address value is assigned to the from parameter and passed to the escrow_deposit_tokens function:

fn escrow_deposit_tokens(
ref self: ContractState,
contract_address: ContractAddress,
from: ContractAddress,
token_ids: Span<u256>,
) {
let to = starknet::get_contract_address();
let erc721 = IERC721Dispatcher { contract_address };
​
let mut i = 0_usize;
loop {
if i == token_ids.len() {
break ();
}
​
let token_id = *token_ids[i];
@> erc721.transfer_from(from, to, token_id);
self.escrow.write((contract_address, token_id), from);
i += 1;
};
}

the function will revert due to the zero address being used in the transfer_from call, effectively rendering the deposit functionality unusable.

Impact

DOS of deposit_tokens function

Tools Used

Implement explicit caller address validation:

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 from = starknet::get_caller_address();
+ assert(!from.is_zero(), 'Invalid caller address');
​
// ...
​
escrow_deposit_tokens(ref self, collection_l2, from, token_ids);
​
// ...
}

Recommendations

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational / Gas

Please, do not suppose impacts, think about the real impact of the bug and check the CodeHawks documentation to confirm: https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity A PoC always helps to understand the real impact possible.

Support

FAQs

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