Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Winner and bidders can double refunds their bids #66

Closed
c4-submissions opened this issue Nov 1, 2023 · 3 comments
Closed

Winner and bidders can double refunds their bids #66

c4-submissions opened this issue Nov 1, 2023 · 3 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-1323 edited-by-warden satisfactory satisfies C4 submission criteria; eligible for awards

Comments

@c4-submissions
Copy link
Contributor

c4-submissions commented Nov 1, 2023

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/AuctionDemo.sol#L125

Vulnerability details

Impact

Due to claimAuction and cancelBid both using = in their time equations, bidders will be able to double refunds their bids.

function claimAuction(...) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
require(block.timestamp >= minter.getAuctionEndTime(_tokenid)...);

function cancelBid(...) public {...
require(block.timestamp <= minter.getAuctionEndTime(_tokenid)...);

Proof of Concept

The winner is able to claim the auction at it's exact auction end time.

function claimAuction(...) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
require(block.timestamp >= minter.getAuctionEndTime(_tokenid)...);

However bidders are also able to cancel their bids at exact auction end time.

function cancelBid(...) public {...
require(block.timestamp <= minter.getAuctionEndTime(_tokenid)...);

This means that the winner can claim, and him or some bidder to double refunds their bids. This is done by first using the refund in claimAuction and then canceling his bid with cancelBid.

A few example scenarios occur, where in order to do this you must have some failed (not winning) bids:

  1. The winner can make multiple bids and either re-enter here after every not winning bid and refund it, or simply schedule in one TX claimAuction and cancelBid on all his not winning bids.

  2. Users can re-enter their failed bids and refunds them.

  3. Seller of the NFT can re-enter and claim his failed bids.

  4. Users can back-run the claimAuction call, and as long as they are with the same timestamp they will be able to cancelBid.

Tools Used

Manual review

Recommended Mitigation Steps

Even if the re-entracy is fixed the winner would still be able to schedule claimAuction and cancelBid in one TX and double refund his other bids. The main issue is <= which needs to be made < in cancelBid.

Assessed type

Invalid Validation

@c4-submissions c4-submissions added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Nov 1, 2023
c4-submissions added a commit that referenced this issue Nov 1, 2023
@c4-pre-sort
Copy link

141345 marked the issue as duplicate of #962

@c4-judge
Copy link

c4-judge commented Dec 4, 2023

alex-ppg marked the issue as duplicate of #1323

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Dec 8, 2023
@c4-judge
Copy link

c4-judge commented Dec 8, 2023

alex-ppg marked the issue as satisfactory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-1323 edited-by-warden satisfactory satisfies C4 submission criteria; eligible for awards
Projects
None yet
Development

No branches or pull requests

4 participants