Owner of the token will not receive the funds of the highest bid after an Auction is claimed

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

In claimAuction function of AuctionDemo.sol, token owner should receive funds, but it will be transferred to AuctionDemo contract's owner.

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

Proof of Concept

I tried to write tests for mintAndAuction and I'm attaching them below.

AuctionDemo.t.sol

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.19;

import {console} from "forge-std/console.sol";
import {DeployHelper} from "./DeployHelper.t.sol";
import {NextGenRandomizerNXT} from "../smart-contracts/RandomizerNXT.sol";
import {auctionDemo} from "../smart-contracts/AuctionDemo.sol";
import {NextGenMinterContract} from "../smart-contracts/MinterContract.sol";
import {IERC721Receiver} from "../smart-contracts/IERC721Receiver.sol";

contract AuctionDemoTest is DeployHelper {

    function setupEnv() public virtual override {
        super.setupEnv();

        // burn collection 0
        vm.startPrank(funcAdminColManager);
        coreContract.createCollection("BurnCol", "", "ColDesc", "", "", "", "", new string[](0));

        // new collection 1
        coreContract.createCollection("NewCol", "", "ColDesc", "", "", "", "", new string[](0));

        vm.stopPrank();
    }
    
    function test_auction() public {

        uint256 collectionID = 2;
        uint256 collectionTotalSupply = 10;
        vm.warp(1 days);
        uint256 auctionEndTime = block.timestamp + 1 days;
        uint allowlistStartTime = block.timestamp;
        uint256 timePeriod = 2 minutes;
        uint256 tokenId;

        // mint and auction
        vm.startPrank(funcAdminColManager);
        vm.deal(funcAdminColManager, 1 ether);
        coreContract.addRandomizer(collectionID, address(new NextGenRandomizerNXT(address(randomPoolContract), address(adminContract), address(coreContract))));
        coreContract.setCollectionData(collectionID, artist, 100, collectionTotalSupply, 0);
        minterContract.setCollectionCosts(collectionID, 0, 0, 0, timePeriod, 0, address(0x0));
        minterContract.setCollectionPhases(collectionID, allowlistStartTime, 0, 0, 0, 0);
        minterContract.mintAndAuction(customer4, "", 0, collectionID, auctionEndTime);
        tokenId = coreContract.viewTokensIndexMin(collectionID) + coreContract.viewCirSupply(collectionID) - 1;
        vm.stopPrank();

        // do auction
        vm.deal(customer1, 1 ether);
        vm.deal(customer2, 1 ether);
        vm.deal(customer3, 1 ether);

        vm.prank(customer1);
        auctionContract.participateToAuction{value: 0.1 ether}(tokenId);
        vm.prank(customer2);
        auctionContract.participateToAuction{value: 0.2 ether}(tokenId);
        vm.prank(customer3);
        auctionContract.participateToAuction{value: 0.4 ether}(tokenId);

        assertEq(customer4.balance, 0, "token owner balance isn't zero");
        assertEq(coreContract.balanceOf(customer4), 1);
        assertEq(coreContract.balanceOf(customer1), 0, "customer1 has some nfts");
        assertEq(coreContract.balanceOf(customer2), 0, "customer2 has some nfts");
        assertEq(coreContract.balanceOf(customer3), 0, "customer3 has some nfts");

        vm.prank(customer4);
        coreContract.approve(address(auctionContract), tokenId);

        vm.warp(minterContract.getAuctionEndTime(tokenId));

        vm.prank(customer3);
        auctionContract.claimAuction(tokenId);

        assertEq(customer1.balance, 1 ether, "1");
        assertEq(coreContract.balanceOf(customer1), 0);
        assertEq(customer2.balance, 1 ether, "2");
        assertEq(coreContract.balanceOf(customer2), 0);
        assertEq(customer3.balance, 0.6 ether, "3");
        assertEq(coreContract.balanceOf(customer3), 1);
        assertEq(customer4.balance, 0.4 ether, "token owner didn't get eth");
        assertEq(coreContract.balanceOf(customer4), 0);
    }
}

DeployHelper.t.sol

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.19;

import "forge-std/Test.sol";
import {NextGenAdmins} from "../smart-contracts/NextGenAdmins.sol";
import {NextGenMinterContract} from "../smart-contracts/MinterContract.sol";
import {NextGenCore} from "../smart-contracts/NextGenCore.sol";
import {auctionDemo} from "../smart-contracts/AuctionDemo.sol";
import {randomPool} from "../smart-contracts/XRandoms.sol";

abstract contract DeployHelper is Test {
    NextGenCore coreContract;
    NextGenAdmins adminContract;
    NextGenMinterContract minterContract;
    randomPool randomPoolContract;
    auctionDemo auctionContract;

    address public admin = vm.addr(0x01);
    address public funcAdminColManager = vm.addr(0x02);
    address public artist = vm.addr(0x04);
    address public customer1 = vm.addr(0x05);
    address public customer2 = vm.addr(0x06);
    address public customer3 = vm.addr(0x07);
    address public customer4 = vm.addr(0x08);

    function setUp() public {
        setupEnv();
    }

    function setupEnv() public virtual {
        vm.startPrank(admin);
        adminContract = new NextGenAdmins();
        coreContract = new NextGenCore("TNextGen", "TNG", address(adminContract));
        minterContract = new NextGenMinterContract(address(coreContract), address(0x0), address(adminContract));
        randomPoolContract = new randomPool();
        coreContract.addMinterContract(address(minterContract));
        auctionContract = new auctionDemo(address(minterContract), address(coreContract), address(adminContract));

        // register collection func admin
        adminContract.registerFunctionAdmin(funcAdminColManager, NextGenCore.createCollection.selector, true);
        adminContract.registerFunctionAdmin(funcAdminColManager, NextGenCore.setCollectionData.selector, true);
        adminContract.registerFunctionAdmin(funcAdminColManager, NextGenMinterContract.setCollectionCosts.selector, true);
        adminContract.registerFunctionAdmin(funcAdminColManager, NextGenMinterContract.setCollectionPhases.selector, true);
        adminContract.registerFunctionAdmin(funcAdminColManager, NextGenCore.addRandomizer.selector, true);
        adminContract.registerFunctionAdmin(funcAdminColManager, NextGenMinterContract.mintAndAuction.selector, true);
        vm.stopPrank();
    }
}

Tools Used

VS Code, Manual Review

Recommended Mitigation Steps

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

Assessed type

Other

[c4-pre-sort]

141345 marked the issue as primary issue

[c4-sponsor]

a2rocket marked the issue as disagree with severity

[a2rocket]

mintAndAuction function can only be called by trusted parties. The _recipient of that function will be a trusted wallet that will also call setApprovalForAll() from the Core contract for the Auction Contract. In our case the _recipient will be the deployer of the Auction contract so at the end of the day the token owner and auction owner are the same person.

[c4-sponsor]

a2rocket (sponsor) disputed

[c4-sponsor]

a2rocket (sponsor) confirmed

[c4-pre-sort]

141345 marked the issue as sufficient quality report

[c4-judge]

alex-ppg marked issue #738 as primary and marked this issue as a duplicate of 738

[c4-judge]

alex-ppg marked the issue as satisfactory

[c4-judge]

alex-ppg changed the severity to 2 (Med Risk)