Contract can be drained from the `claimAuction` contract

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

Due to the way the claimAuction function is written, it is also possible to drain the contract when sending bids back to users. This can only happen if the highestBidder made multiple bids which is likely.

Proof of Concept

In this section of the claimAuction 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 function (bool success, ) = payable(owner()).call{value: highestBid}(""); specifically transfers the highestBid every time the highestBidder bids is called in the loop.

So for a case where the highestBidder bids 0.5Eth, 0.7Eth, 0.8Eth, 1.2Eth bids and 1.5Eth as the winning bid for the auction, the claimAuction function will transfer our 1.5 * 5 out of the contract.

In a best-case scenario, the contract is drained to the owner wallet, and in the worst-case scenario it'll be impossible to claim the auction NFT in a case where total of Eth left is not up to 7.5Eth,

Tools Used

Manual Review

Recommended Mitigation Steps

• check for each bids and send the amount of each bids just as done in the else if statement

Assessed type

ETH-Transfer

[c4-pre-sort]

141345 marked the issue as duplicate of #962

[c4-judge]

alex-ppg marked the issue as not a duplicate

[alex-ppg]

The Warden specifies that if the highest bidder is the bidder of multiple bids, the claimAuction loop will incorrectly distribute the highestBid to the owner of the contract multiple times.

This submission is incorrect as the Warden has failed to identify that the if clause which protects the owner transfer's execution strictly evaluates that the highestBid of the overall auction is equal to the bid that is being processed. Coupled with the fact that bids are mandated to be greater than the previous one and canceled bids aren't processed, this submission is incorrect.

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid

[c4-judge]

alex-ppg marked the issue as primary issue