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

Frontrunning the highest winning bid and having users funds permanently blocked can happen by bidding in the same block as the wining claim #92

Closed
c4-submissions opened this issue Nov 1, 2023 · 5 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-175 partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%)

Comments

@c4-submissions
Copy link
Contributor

Lines of code

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

Vulnerability details

Summary

Biding in an auction (via auctionDemo::participateToAuction) and claiming the NFT at the end of an auction (via auctionDemo::claimAuction) can be called in the same block. This introduces 2 major issues:

  • Exactly in the block when an auction is finished, a user may frontrun the call to claimAuction with his own participateToAuction (or simply call participateToAuction directly it nobody called claimAuction) and win simply by adding 1 WEI over the previous las winning bid, then himself call claimAuction in the same block and retrieve the NFT. Thus winning the NFT auction by 1 WEI.
  • If a normal user bids in the same block when claimAuction is called but his transaction is after the claimAuction, it will not revert and he will lose his funds forever since after that block, the cancel bids functions will not work any more

Vulnerability Details

After an auction has started users can bid using auctionDemo::participateToAuction and when the auction is finished, the winner or admin can call auctionDemo::claimAuction to refund non-winners and send the NFT to the winner.

auctionDemo::participateToAuction and auctionDemo::claimAuction can be called in the same block because:

thus in the moment block.timestamp == minter.getAuctionEndTime(_tokenid) both can be called.

cancelAllBids and cancelBid also check as so block.timestamp <= minter.getAuctionEndTime(_tokenid), to note they will not be callable after the end time has passed, making bids after the claimAuction

Impact

NFT winning bid may be frontrun and surpassed with 1 WEI and users may have ETH forever blocked in the contract.

Tools Used

Manual review

Recommendations

Do not allow participateToAuction and claimAuction to be called at the same time.

Assessed type

Timing

@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 2, 2023

alex-ppg marked the issue as not a duplicate

@c4-judge
Copy link

c4-judge commented Dec 2, 2023

alex-ppg marked the issue as duplicate of #1926

@c4-judge
Copy link

c4-judge commented Dec 8, 2023

alex-ppg marked the issue as partial-50

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

c4-judge commented Dec 9, 2023

alex-ppg changed the severity to 2 (Med Risk)

@c4-judge c4-judge added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value downgraded by judge Judge downgraded the risk level of this issue and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Dec 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-175 partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%)
Projects
None yet
Development

No branches or pull requests

3 participants