Starknet Auction

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

Incorrect User Balance Update in bid function Allows Loss of User Funds

Summary

The current implementation of the bidding mechanism in the auction contract fails to correctly update the user's balance when they place multiple bids. Specifically, when a user places a bid, the new bid amount overwrites the existing bid amount in storage, rather than being accumulated (added to the previous amount). This creates a critical vulnerability that can result in loss of user funds and incorrect auction behavior.

Vulnerability Details

In the auction contract, users can place multiple bids during the bidding period. The current implementation does not correctly handle this scenario because it fails to accumulate the user's bids.

self.bid_values.entry(sender).write(amount);

Impact

In this line, the user's bid is being overwritten with the new bid amount (amount). This overwriting behavior causes the following issues:

  1. Loss of Funds: If a user places multiple bids, their previous bids are effectively lost. The contract only remembers the most recent bid, disregarding all previous bids.

  2. Incorrect Total Commitment: The contract cannot accurately track the total funds a user has committed to the auction, which could lead to incorrect balances when refunding or awarding the highest bid.

Example

  • User A places an initial bid of 5 tokens.

  • User A later places a second bid of 10 tokens.

  • The contract overwrites the first bid, so only 10 tokens are recorded, even though User A has actually bid a total of 15 tokens.

This leads to incorrect tracking of User A's total commitment to the auction and potential loss of 5 tokens (the first bid), which the contract no longer recognizes.


Exploit Scenario:

  1. Multiple Bids from a User:

    • A user bids multiple times during an auction.

    • Due to the contract overwriting the previous bid amount, only the last bid is stored.

    • When the auction ends, the user's actual total bid is not reflected, leading to loss of funds already committed.

  2. Malicious Exploit:

    • A malicious user could deliberately place multiple small bids to manipulate the system.

    • The contract will only store the last bid, allowing the attacker to manipulate the final outcome and potentially avoid committing the full bid amount.


Impact on the Auction:

  • Funds Loss: Users who place multiple bids lose any funds associated with their earlier bids because the contract does not track the total amount committed.

  • Auction Inaccuracy: The highest bid may be incorrectly recorded, resulting in unfair or incorrect auction outcomes.

  • Security Breach: This could lead to potential exploits, where malicious users manipulate the system to bypass bidding constraints.

Tools Used

Manual Review

Recommendation

To fix this vulnerability, the user's bid should be accumulated rather than overwritten. The total bid should be updated as follows:

let previous_bid = self.bid_values.entry(sender).read();
self.bid_values.entry(sender).write(previous_bid + amount);

By adding the new bid to the existing balance (+=), the contract ensures that the user's total bid is properly tracked, preventing any loss of funds and ensuring fair auction outcomes.


Conclusion:

The failure to correctly update user balances in the auction contract introduces a high-severity vulnerability. This could result in the loss of user funds and incorrect behavior in the auction process. Immediate corrective action is required to ensure that bids are properly accumulated, ensuring fairness and security in the auction.

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.