c4-submissions commented 7 months ago

Lines of code

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

Vulnerability details

Description

claimAuction is used to redeem the auction's ERC-721 and refund all bidders that didn't win the auction. In this process, callbacks are sent to every single bidder via low-level calls (that triggers fallbacks/receives) and ERC721.safeTransferFrom. So, every bidder has an access to a callback when claimAuction is called, which makes possible to any of them to perma-revert the function via gas-griefing.

Proof of Concept

Auction has ended and the winner calls claimAuction:

function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
    require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);
    auctionClaim[_tokenid] = true;
    uint256 highestBid = returnHighestBid(_tokenid);
    address ownerOfToken = IERC721(gencore).ownerOf(_tokenid);
    address highestBidder = returnHighestBidder(_tokenid);
    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 {}
    }
}

If an attacker is a bidder that lost the auction, he needs to be refunded. So, the following line will call him:

(bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");

If attacker is a smart contract, fallback/receive will be triggered to handle the received ether. In this callback, attacker can gas-grief returning a very long string:
```
fallback() external payable {
if (gasgrief) {
    return VERY_LONG_STRING; 
}
}
```
- The caller doesn't assign a variable to the return value, but in solidity it is stored anyway in caller's memory when low-level call is used, which makes the gas-grief possible.
The gas-grief will revert claimAuction, potentially perma-locking all the funds and the prize in the contract, because there are no other function to claim the refunds or the prize. Also, there is no emergency withdraw function.
If luckily there weren't any auction's loser trying to gas-grief, the winner can also denial of service the function if he wants. The line IERC721(gencore).safeTransferFrom(ownerOfToken, highestBidder, _tokenid); triggers a callback, which winner can use to revert:
```
function onERC721Received(
    address operator,
    address from,
    uint256 tokenId,
    bytes calldata data
) external returns (bytes4) {
    if (gasgrief) {
        revert();
    }}
```

Impact

Any bidder can lock all the funds and the prize forever.

Likelihood: High, this attack can always be done in any auction. An attacker just needs to be a bidder, which he can do spending dust amounts if he is the first bidder.
Impact: High, the attack will always perma-lock all funds, the winner's prize and there isn't any emergency withdraw function to rescue from the attack.
- Risk: High (High Likelihood + High Impact)

Tools Used

Manual Review

Recommended Mitigation Steps

Claims and refunds needs to be individual, so add a function to individually refund and make claimAuction a function that only transfers the ERC-721.

Assessed type

DoS

c4-pre-sort commented 7 months ago

141345 marked the issue as duplicate of #1632

c4-pre-sort commented 7 months ago

141345 marked the issue as duplicate of #843

c4-pre-sort commented 7 months ago

141345 marked the issue as duplicate of #486