Starknet Auction
First Flight #26
Beginner FriendlyNFT
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

The NFT token_id is declared as a u64 which means Starknet Auction cannot support NFTs that use a token_id requiring a u256

poolig4n

Summary

The NFT token_id used in the Starknet Auction contract is declared as u64 rather than u256, as can be seen on line 52 of the StarknetAuction contract in the file: starknet_auction.cairo

#[storage]
struct Storage {
nft_id: u64, <@--- should be u256

The ERC standard states that this value should be u256 as can be seen here.

https://ethereum.org/en/developers/docs/standards/tokens/erc-721/

The Scarb Starknet interface also declares this value to be u256

#[starknet::interface]
pub trait IERC721<TState> {
fn balance_of(self: @TState, account: ContractAddress) -> u256;
fn owner_of(self: @TState, token_id: u256) -> ContractAddress;
fn safe_transfer_from(
ref self: TState,
from: ContractAddress,
to: ContractAddress,
token_id: u256,
data: Span<felt252>
);
fn transfer_from(ref self: TState, from: ContractAddress, to: ContractAddress, token_id: u256);
fn approve(ref self: TState, to: ContractAddress, token_id: u256);
fn set_approval_for_all(ref self: TState, operator: ContractAddress, approved: bool);
fn get_approved(self: @TState, token_id: u256) -> ContractAddress;
fn is_approved_for_all(
self: @TState, owner: ContractAddress, operator: ContractAddress
) -> bool;
}

Vulnerability:

The issue lies in the fact that it's possible for an NFT token_id to be larger than the u64 storage allows for meaning any NFT with a token_id requiring u256 cannot be used with the Starknet Auction protocol.

POC:

Change the file Mock Token as shown below, (mock_erc721_token.cairo) so the constructor uses a value of: 1_u256

#[constructor]
fn constructor(
ref self: ContractState,
recipient: ContractAddress
) {
let token_id = 1_u256; //1; <@--- This line was 1 and is now 1_u256
self.erc721.initializer("MyToken", "MTK", "");
self.erc721.mint(recipient, token_id);
}

In the test case if we set the nft_id to 1_u256 as shown below, the constructor throws an error because there is a type mismatch.

let nft_id = 1_u256;
println!("Attempting to create contract");
(erc20_contract_address, erc721_contract_address, nft_id, ).serialize(ref calldata);
// Deploy the Auction contract.
let (contract_address, _) = contract.deploy(@calldata).unwrap();
println!("Contract created"); <---@ This line is never reached
​

The output of the test case is shown below.

Collected 1 test(s) from starknet_auction package
Running 0 test(s) from src/
Running 1 test(s) from tests/
Attempting to create contract
[FAIL] starknet_auction_integrationtest::test_client::test_client
​
Failure data:
"Hint Error:
0x496e70757420746f6f206c6f6e6720666f7220617267756d656e7473 ('Input too long for arguments')
"
​
Tests: 0 passed, 1 failed, 0 skipped, 0 ignored, 3 filtered out

References:

Updates

Lead Judging Commences

bube Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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