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

safeTransfer denial of service in claimAuction function #1075

Closed
c4-submissions opened this issue Nov 11, 2023 · 4 comments
Closed

safeTransfer denial of service in claimAuction function #1075

c4-submissions opened this issue Nov 11, 2023 · 4 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 duplicate-739 edited-by-warden partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%)

Comments

@c4-submissions
Copy link
Contributor

c4-submissions commented Nov 11, 2023

Lines of code

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

Vulnerability details

Impact

A malicious receiver can forcibly revert transactions by reverting inside onERC721Received() or by using a loop to consume all the gas. Protocol should not assume that safeTransferFrom to an arbitrary address will succeed.

Proof of Concept

A user can forcibly revert the call in safeTransferFrom function:

 for (uint256 i=0; i< auctionInfoData[_tokenid].length; i ++) {
            if (auctionInfoData[_tokenid][i].bidder == highestBidder && auctionInfoData[_tokenid][i].bid == highestBid && auctionInfoData[_tokenid][i].status == true) {
                IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid);
                (bool success, ) = payable(owner()).call{value: highestBid}("");
                emit ClaimAuction(owner(), _tokenid, success, highestBid);
            } else if (auctionInfoData[_tokenid][i].status == true) {
                (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");
                emit Refund(auctionInfoData[_tokenid][i].bidder, _tokenid, success, highestBid);
            } else {}

This may lead to the user not being able to get the NFT that he won in the auction , but this will lead to DOS -attack and none of the remaining bidders won't be able to withdraw the funds stuck in the contract because there is no other function for them to use to get back their bids AFTER getAuctionEndTime:

 function cancelBid(uint256 _tokenid, uint256 index) public {
        require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");
   function cancelAllBids(uint256 _tokenid) public {
        require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");

Tools Used

Manual review

Recommended Mitigation Steps

Implement pull over push.

Assessed type

DoS

@c4-submissions c4-submissions added 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 labels Nov 11, 2023
c4-submissions added a commit that referenced this issue Nov 11, 2023
@c4-pre-sort
Copy link

141345 marked the issue as duplicate of #486

@c4-judge c4-judge reopened this Dec 1, 2023
@c4-judge
Copy link

c4-judge commented Dec 1, 2023

alex-ppg marked the issue as not a duplicate

@c4-judge c4-judge closed this as completed Dec 1, 2023
@c4-judge
Copy link

c4-judge commented Dec 1, 2023

alex-ppg marked the issue as duplicate of #1759

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

c4-judge commented Dec 8, 2023

alex-ppg marked the issue as partial-50

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 duplicate-739 edited-by-warden partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%)
Projects
None yet
Development

No branches or pull requests

4 participants