Missing highBid value update returnHighestBidder would return the wrong HighestBidder causing nft to be minted to the wrong winner

[thebrittfactor]

Lines of code

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

Vulnerability details

Impact

returnHighestBidder is used in checking who the winner is, however the code implemented in doing this was flawed as it missed updating highBid variable for the current higest bid found so far before next iteration. Missing this would cost function to simply return whoever was the last valid bidder which is not what's intended

Proof of Concept

AuctionDemo:returnHighestBidder()

function returnHighestBidder(uint256 _tokenid) public view returns
(address) {
        uint256 highBid = 0;
        uint256 index;
        for (uint256 i=0; i< auctionInfoData[_tokenid].length; i++) {
            if (auctionInfoData[_tokenid][i].bid > highBid &&
auctionInfoData[_tokenid][i].status == true) {
                index = i; // @audit-issue missing highBid update
            }
        }
        if (auctionInfoData[_tokenid][index].status == true) {
                return auctionInfoData[_tokenid][index].bidder;
            } else {
                revert("No Active Bidder");
        }
    }

Tools Used

Manaul Review

Recommended Mitigation Steps

Change function to account for the bid amount

function returnHighestBidder(uint256 _tokenid) public view returns
(address) {
        uint256 highBid = 0;
        uint256 index;
        for (uint256 i=0; i< auctionInfoData[_tokenid].length; i++) {
            if (auctionInfoData[_tokenid][i].bid > highBid &&
auctionInfoData[_tokenid][i].status == true) {
                index = i;
              + highBid = auctionInfoData[_tokenid][i].bid;
            }
        }
        if (auctionInfoData[_tokenid][index].status == true) {
                return auctionInfoData[_tokenid][index].bidder;
            } else {
                revert("No Active Bidder");
        }
    }

Assessed type

Other

[thebrittfactor]

For transparency, due to submission issues, the warden provided this submission prior to audit close.

[c4-pre-sort]

141345 marked the issue as duplicate of #586

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid