Starknet Auction

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

Multiple user withdrawals allowed without resetting bid amount in `withdraw`.

Vulnerability Details:

The withdraw function in the auction contract allows multiple withdrawals, due to the failure to reset user's bid amounts after a successful withdrawal. As a result, a user can repeatedly call the withdraw function and drain the contract’s funds.

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});
}

Once a user calls withdraw, the amount they previously bid is transferred to their account. However, since their entry in the bid_values mapping is not updated to 0, they can call the withdraw function again and keep withdrawing the same amount.

Impact:

This vulnerability is high severity because it enables malicious actors to drain the contract’s funds. By repeatedly withdrawing their bid amount without it being reset, they can take more tokens than they initially bid.

Tools Used

Manual Code Review, VSCode

Recommendations:

To prevent users from withdrawing multiple times, reset their bid amount in the bid_values mapping to 0 after a successful withdrawal:

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();
+ self.bid_values.entry(caller).write(0);
erc20_dispatcher.transfer_from(sender, caller, amount.into());
}
self.emit(Withdraw {amount: amount, caller: caller});
}
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.