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

A bidder who makes multiple bids can only withdraw their most recent bid amount resulting in them losing tokens they'd previously bid.

poolig4n

Description
Due to how the internal tracking of bids is implemented with a ContractAddress to amount map only the last bid amount is stored for a Bidder using the same ContractAddress. This means if:

Bidder1 bids the following 3 bids:

  • 1

  • 10

  • 30

Then Bidder2 bids the following amount; then the auction finishes and so Bidder2 is the winner:

  • 31

The total in the contract is 31 + 30 + 10 + 1 = 72
The NFT / Auction owner withdraws 31 leaving 41.
Bidder1 attempts to withdraw their failed bids, expecting to get 41 tokens refunded, but only receives 30 tokens which is their most recent bid amount.

NOTE: The example above is a contrived example used to simplify the explanation of the issue; typically Bidder1 would not bid against themselves.

The issue lies with the fact the map shown below only tracks the last bid amount not the total of the bids for the Bidder/ ContractAddress.

bid_values: Map<ContractAddress, u64>

Impact
Bidder's funds are potentially trapped in the contract and they cannot legitimately withdraw them.

Proof of Concept
Within the bid function this line(in red) only writes the current bid (amount) into the bid_values map where we need to get the current amount that has been bid previously and add that to the new bid amount.

Something like the following is indicative of a potential solution (this code is untested and should be considered purely as an example of a possible implementation)

The bid function contains the following code (in red), where amount is the current amount being bid in the transaction.

- self.bid_values.entry(sender).write(amount);
+ let current_amount = self.bid_values.entry(sender).read();
+ let total_amount_bid = current_amount + amount;
+ self.bid_values.entry(sender).write(total_amount_bid);

Recommended mitigation

  • Improve bid tracking and accounting logic such that all user bids are tracked and can be withdrawn.

References
N/A

Tools Used

  • Manual Review

Updates

Lead Judging Commences

bube Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Wrong bid amount in `bid` function

In the `bid` function the bid values are stored using `self.bid_values.entry(sender).write(amount)` directly, but this overwrites any previous bids made by the same bidder. Therefore if a participant makes 2 or more bids, the participant can then withdraw only the last value of the last bid. That is incorrect, the protocol should save all bids and a participant should withdraw the value of the all unsuccessful bids.

Support

FAQs

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