NextGen - Viktor_Cortess's results

Lines of code

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

Vulnerability details

Impact

Reentrancy in the mint function allows users from allowlist to bypass amount restrictions.

Proof of Concept

During the minting process in phase 1, the following line of the code in the mint() function checks how many tokens the user from allowlist has minted and if this amount is not more than allowed:

213,217: require(_maxAllowance >= gencore.retrieveTokensMintedALPerAddress(col, msg.sender) + _numberOfTokens, "AL limit");

The issue is that the rewriting of the number of tokens minted per address is made after the call to the minting function, allowing the receiver of the NFT to call the mint function from the Minter Contract again without changes in the mapping tokensMintedAllowlistAddress:

function mint(uint256 mintIndex, address _mintingAddress , address _mintTo, string memory _tokenData, uint256 _saltfun_o, uint256 _collectionID, uint256 phase) external {
    require(msg.sender == minterContract, "Caller is not the Minter Contract");
    collectionAdditionalData[_collectionID].collectionCirculationSupply = collectionAdditionalData[_collectionID].collectionCirculationSupply + 1;
    if (collectionAdditionalData[_collectionID].collectionTotalSupply >= collectionAdditionalData[_collectionID].collectionCirculationSupply) {
        _mintProcessing(mintIndex, _mintTo, _tokenData, _collectionID, _saltfun_o); 
        if (phase == 1) { //@audit here
            tokensMintedAllowlistAddress[_collectionID][_mintingAddress] = tokensMintedAllowlistAddress[_collectionID][_mintingAddress] + 1;
        } else {
            tokensMintedPerAddress[_collectionID][_mintingAddress] = tokensMintedPerAddress[_collectionID][_mintingAddress] + 1;
        }
    }
}

As a result the gencore.retrieveTokensMintedALPerAddress(col, msg.sender) will return 0 allowing the user endless minting.

POC for phase 1 is here:

https://gist.github.com/viktorcortess/381f37cf04173528b679fb1f454aedaa

The similar situation with phase 2. Require statements check if gencore.retrieveTokensMintedPublicPerAddress(col, msg.sender) + _numberOfTokens <= gencore.viewMaxAllowance(col) :

 } else if (block.timestamp >= collectionPhases[col].publicStartTime && block.timestamp <= collectionPhases[col].publicEndTime) {
        phase = 2;
        console.log("publicStartTime");
        require(_numberOfTokens <= gencore.viewMaxAllowance(col), "Change no of tokens");
        require(gencore.retrieveTokensMintedPublicPerAddress(col, msg.sender) + _numberOfTokens <= gencore.viewMaxAllowance(col), "Max"); //@Mid look here
        mintingAddress = msg.sender;
        tokData = '"public"';
    }

But the tokensMintedPerAddress mapping is rewritten after the call to the _mintProcessing:

if (collectionAdditionalData[_collectionID].collectionTotalSupply >= collectionAdditionalData[_collectionID].collectionCirculationSupply) {
        
        _mintProcessing(mintIndex, _mintTo, _tokenData, _collectionID, _saltfun_o);
        if (phase == 1) {
            console.log("phase count");
            tokensMintedAllowlistAddress[_collectionID][_mintingAddress] = tokensMintedAllowlistAddress[_collectionID][_mintingAddress] + 1;
            console.log("tokensMintedAllowlistAddress[_collectionID][_mintingAddress]",tokensMintedAllowlistAddress[_collectionID][_mintingAddress]);
        } else {
            tokensMintedPerAddress[_collectionID][_mintingAddress] = tokensMintedPerAddress[_collectionID][_mintingAddress] + 1;
        }
    }

Tools Used

vs

Recommended Mitigation Steps

The rewriting of variables should be performed before calling the _mintProcessing function:

  if (phase == 1) {
            tokensMintedAllowlistAddress[_collectionID][_mintingAddress] = tokensMintedAllowlistAddress[_collectionID][_mintingAddress] + 1;
        } else {
            tokensMintedPerAddress[_collectionID][_mintingAddress] = tokensMintedPerAddress[_collectionID][_mintingAddress] + 1;
        }

_mintProcessing(mintIndex, _mintTo, _tokenData, _collectionID, _saltfun_o);
      

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

Function claimAuction() uses a loop for sending ether to the auction participants. This makes it prone to a DDoS attack blocking users from receiving of funds.

Proof of Concept

After the auction is ended winner or admin should call the function claimAuction() where ether is sent within the loop:

function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){ //@audit who pays for gas
    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 {}
    }
}

The problem is that contracts also can participate in the auction. And if one of these contracts implements an endless loop in its receive fallback this will lead to the stuck transaction:

contract RevertOnReceive {
uint256 x;
receive() external payable {
    // This contract will revert on any incoming ether transfer
     while (true) {
        x = x + 1 ;
        // This creates an endless loop
    }
}
}

Tools Used

vs

Recommended Mitigation Steps

Avoid sending funds in a loop to unknown addresses. Consider implementing logic where every user withdraws his personal funds.

Assessed type

Loop

Lines of code

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

Vulnerability details

Impact

When claiming an auction the token is transferred from the owner of the token to the winner but the highest bid i.e. the funds received for the token's sale is sent to the owner of the auction contract, not the owner of the token.

Proof of Concept

A quote from the contests's invariants:

The highest bidder will receive the token after an auction finishes, the owner of the token will receive the funds and all other participants will get refunded.

Function claimAuction() transfers the token from its owner to the winner and returns the funds of other participants. But the highest bid goes to the owner of the auction contract not the owner of the token.

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

Tools Used

vs

Recommended Mitigation Steps

Variable ownerOfToken should be used:

(bool success, ) = payable(ownerOfToken).call{value: highestBid}("");

Assessed type

Context