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

Highest Bidder Can Withdraw Funds After Receiving NFT

PureSecurity

Summary

A critical vulnerability in the StarkNet auction contract allows the highest bidder to withdraw their funds after receiving the NFT if they invoke the withdraw function before the contract owner calls withdraw. This enables the highest bidder to receive the NFT and reclaim their funds, resulting in no payment for the contract owner.

Vulnerability Details

The highest bidder’s bid is stored both in the highest_bid field and the bid_values map. After the auction ends, the highest bidder can still invoke the withdraw function because their bid is also stored in bid_values. This allows the highest bidder to withdraw their funds after receiving the NFT by calling end.

Sequence of Events:

  1. The contract owner calls end to finalize the auction and transfer the NFT to the highest bidder.

  2. The highest bidder receives the NFT.

  3. The highest bidder then invokes withdraw and reclaims their bid from bid_values.

  4. This results in the highest bidder having both the NFT and their bid, leaving the contract owner without the proceeds from the auction.

Impact

  • Double benefit for the highest bidder: The highest bidder can receive the NFT and reclaim their bid, which allows them to win the auction without paying.

  • Loss for the contract owner: The contract owner will not receive the highest bid, leading to financial loss as the auction's purpose is defeated.

  • High exploitability: The vulnerability is easily exploitable by any highest bidder, making it a high-severity issue.

Tools Used

Manual code review.

Recommendations

To fix this issue, it is essential to prevent the highest bidder from calling the withdraw function after they have won the auction. A check must be added in the withdraw function to ensure that the highest bidder cannot withdraw their bid after the auction has ended.

Recommended Fix in Cairo:

fn withdraw(ref self: ContractState) {
assert(self.started.read(), 'Auction is not started');
assert(self.ended.read(), 'Auction is not ended');
let caller = get_caller_address();
let sender = get_contract_address();
let erc20_dispatcher = IERC20Dispatcher { contract_address: self.erc20_token.read() };
let amount = self.bid_values.entry(caller).read();
let amount_owner = self.highest_bid.read();
// Prevent the highest bidder from withdrawing their funds
assert(caller != self.highest_bidder.read(), "Highest bidder cannot withdraw their bid");
​
if caller == self.nft_owner.read() {
self.highest_bid.write(0);
erc20_dispatcher.transfer_from(sender, caller, amount_owner.into());
}
​
if amount > 0 {
self.bid_values.entry(caller).write(0); // Ensure bid is reset
erc20_dispatcher.transfer_from(sender, caller, amount.into());
}
​
self.emit(Withdraw {amount: amount, caller: caller});
}

Explanation:

  1. Check for the highest bidder: The line assert(caller != self.highest_bidder.read(), "Highest bidder cannot withdraw their bid") ensures that the highest bidder cannot invoke withdraw after winning the auction.

  2. Reset bid after withdrawal: In the case of other participants, their bid is reset in bid_values after they withdraw their funds, ensuring no repeated 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.