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

Client can send two or more bit's.

eth0x

Summary

The bid function currently lacks code to track the number of bids placed by each client.

Because of this, a client can place two or more bids.

Vulnerability Details

Clients can place multiple bids in the protocol, but only the latest bid's amount will be stored in the bit_values map. Due to this vulnerability, there will be a discrepancy between the client's token balance and the amount recorded in bit_values. This mismatch can lead to potential issues and exploitation within the protocol.

fn bid(ref self: ContractState, amount: u64) {
let time = get_block_timestamp();
let erc20_dispatcher = IERC20Dispatcher { contract_address: self.erc20_token.read() };
let sender = get_caller_address();
let receiver = get_contract_address();
let current_bid = self.highest_bid.read();
​
assert(self.started.read(), 'Auction is not started');
assert(time < self.bidding_end.read(), 'Auction ended');
assert(amount > current_bid, 'The bid is not sufficient');
self.bid_values.entry(sender).write(amount);
​
// @audit event is before highest_bidder and highest_bid setters should be at the end of the function
self.emit(NewHighestBid {amount: self.highest_bid.read(), sender: sender});
self.highest_bidder.write(sender);
self.highest_bid.write(amount);
erc20_dispatcher.transfer(receiver, amount.into());
}

Impact

Clients cannot withdraw their entire funds from the protocol due to the current limitations in the implementation. This constraint may lead to incomplete or mismatched transactions, affecting the reliability and integrity of the protocol.

Tools Used

manual review

Recommendations

Let's rethink the protocol with improvements to enhance security and functionality:

  1. State Management: Implement enums to manage auction states (e.g., NotStarted, Active, Ended) rather than boolean flags. This avoids confusion and enhances readability.

  2. Reentrancy Guard: Use a reentrancy guard in the withdraw function to prevent reentrancy attacks.

  3. Tracking Bids: Ensure each bid is tracked. Implement logic to handle multiple bids from the same user.

  4. Secure Timestamp: Instead of relying on block.timestamp, consider using a trusted oracle for timekeeping.

  5. Balance Management: Update balances correctly before transferring funds to avoid inconsistencies.

Here’s a revised version of the key functions:

​
fn bid(ref self: ContractState, amount: u64) {\
let time = get\_block\_timestamp();\
let erc20\_dispatcher = IERC20Dispatcher { contract\_address: self.erc20\_token.read() };\
let sender = get\_caller\_address();\
let receiver = get\_contract\_address();\
let current\_bid = self.highest\_bid.read();
assert(self.started.read(), 'Auction is not started');
assert(time < self.bidding_end.read(), 'Auction ended');
assert(amount > current_bid, 'The bid is not sufficient');
// @audit this lines
let amountFromMap = self.bid_values.entry(caller).read();
​
amountFromMap += amount;
self.bid_values.entry(sender).write(amountFromMap);
self.emit(NewHighestBid {amount: self.highest_bid.read(), sender: sender});
self.highest_bidder.write(sender);
self.highest_bid.write(amount);
erc20_dispatcher.transfer(receiver, amount.into());
}

or if not

​
fn bid(ref self: ContractState, amount: u64) {\
let time = get\_block\_timestamp();\
let erc20\_dispatcher = IERC20Dispatcher { contract\_address: self.erc20\_token.read() };\
let sender = get\_caller\_address();\
let receiver = get\_contract\_address();\
let current\_bid = self.highest\_bid.read();
​
assert(self.started.read(), 'Auction is not started');
assert(time < self.bidding_end.read(), 'Auction ended');
assert(amount > current_bid, 'The bid is not sufficient');
​
// @audit this lines
assert(self.bid_values.entry(caller).read() >0, 'Only one bit is possible');
self.bid_values.entry(sender).write(amount);
​
self.emit(NewHighestBid {amount: self.highest_bid.read(), sender: sender});
self.highest_bidder.write(sender);
self.highest_bid.write(amount);
erc20_dispatcher.transfer(receiver, amount.into());
}
Updates

Lead Judging Commences

bube Lead Judge 7 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.