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

Should not enable claiming at auction end time #1195

Closed
c4-submissions opened this issue Nov 12, 2023 · 5 comments
Closed

Should not enable claiming at auction end time #1195

c4-submissions opened this issue Nov 12, 2023 · 5 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-1323 partial-25 Incomplete articulation of vulnerability; eligible for partial credit only (25%)

Comments

@c4-submissions
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L124
https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L104

Vulnerability details

Impact

Should not enable claiming at auction end time, or the winner can claim token without pay the ETH.

Proof of Concept

There are 2 phases in an entire auction, the bidding phase and claiming phase.

In bidding phase, bidders place bids if they want to participate, or cancel bids if they want to leave;
In claiming phase, winner claiming the token and the ETH will be returned to other bidders.

Unfortunately, if we look carefully at cancelBid and claimAuction, we can find that there is time overlap between the 2 phases:

        // bidding phase
        require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");
        // claiming phase
        require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);

Both claiming and canceling can be done at auctionEndTime, this means the winner may first claim the token and then cancel the winning bid to get ETH back at the time point.

Tools Used

Manual Review

Recommended Mitigation Steps

To mitigate this issue, please remove the time overlap and only allow claiming after auctionEndTime.

Assessed type

ERC721

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

141345 marked the issue as duplicate of #962

@c4-judge
Copy link

c4-judge commented Dec 1, 2023

alex-ppg marked the issue as not a duplicate

@c4-judge
Copy link

c4-judge commented Dec 1, 2023

alex-ppg marked the issue as duplicate of #1788

@c4-judge c4-judge added duplicate-1323 partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%) and removed duplicate-1788 labels Dec 4, 2023
@c4-judge
Copy link

c4-judge commented Dec 8, 2023

alex-ppg marked the issue as partial-50

@c4-judge c4-judge added partial-25 Incomplete articulation of vulnerability; eligible for partial credit only (25%) and removed partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%) labels Dec 8, 2023
@c4-judge
Copy link

c4-judge commented Dec 8, 2023

alex-ppg marked the issue as partial-25

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 partial-25 Incomplete articulation of vulnerability; eligible for partial credit only (25%)
Projects
None yet
Development

No branches or pull requests

3 participants