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

Missing State Update for User's Bid Balance

shellh4wks

Missing State Update for User's Bid Balance

In the withdraw function, after transferring the user's bid amount, the user's bid balance in storage is not updated to 0. This leads to a logical vulnerability where users (including attackers) can withdraw repeatedly, because the balance never changes.

Code Path: Where the Vulnerability Occurs

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();
if caller == self.nft_owner.read() {
self.highest_bid.write(0);
erc20_dispatcher.transfer_from(sender, caller, amount_owner.into());
}
​
if amount > 0 {
let sender = get_contract_address();
erc20_dispatcher.transfer_from(sender, caller, amount.into());
}
​
self.emit(Withdraw {amount: amount, caller: caller});
}

In this logic:

  1. The amount for the user is read from storage.

  2. The contract sends the tokens using transfer_from().

  3. However, the state is never updated to set the bid amount to 0 after the transfer.

Impact

  • The user (or an attacker) can call withdraw multiple times to withdraw the same amount repeatedly.

  • This can lead to a complete drain of tokens from the contract.

How to Fix the Vulnerability

After transferring the tokens, the user's bid amount should be updated to 0 to prevent multiple withdrawals.

Here’s the corrected code:

if amount > 0 {
self.bid_values.entry(caller).write(0); // Clear the bid amount first
erc20_dispatcher.transfer_from(sender, caller, amount.into()); // Transfer tokens
}

This ensures that the user’s bid amount is set to 0 immediately, making further withdrawals impossible.

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.

Support

FAQs

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