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

Users can mint more tokens compared to what its allowed to mint #1859

Closed
c4-submissions opened this issue Nov 13, 2023 · 5 comments
Closed
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-1517 satisfactory satisfies C4 submission criteria; eligible for awards

Comments

@c4-submissions
Copy link
Contributor

Lines of code

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

Vulnerability details

Impact

An allowlist address can mint more tokens than what it is allowed to mint and any address during the public phase can mint more tokens compared to what its allowed to mint (maxCollectionPurchases) due to reentrancy into the NextGenMinterContract.mint function.

Proof of Concept

Suppose a user is allowed to mint 3 tokens. Then the user calls the NextGenMinterContract.mint function with _numberOfTokens equals 3. When the protocol makes the onERC721Received invoke from the _safeMint the user reenters into the NextGenMinterContract.mint function with _numberOfTokens equals 2 and on the next step 1. The condition will be true for every call:

213                require(_maxAllowance >= gencore.retrieveTokensMintedALPerAddress(col, _delegator) + _numberOfTokens, "AL limit");

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/MinterContract.sol#L213
After the last reentrance call is executed the previous call continues minting remaining tokens in the loop:

        for(uint256 i = 0; i < _numberOfTokens; i++) {
            uint256 mintIndex = gencore.viewTokensIndexMin(col) + gencore.viewCirSupply(col);
            gencore.mint(mintIndex, mintingAddress, _mintTo, tokData, _saltfun_o, col, phase);
        }

https://github.com/code-423n4/2023-10-nextgen/blob/8b518196629faa37eae39736837b24926fd3c07c/smart-contracts/MinterContract.sol#L234-L237
This way the user receives 6 tokens instead of allowed 3.

The same attack can be provided to mint more tokens during the public phase.

Tools Used

Manual review

Recommended Mitigation Steps

Consider using the nonReentrant modifier to prevent reentrancy in the mint function.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
contract ReEntrancyGuard {
    bool internal locked;
    modifier noReentrant() {
        require(!locked, "No re-entrancy");
        locked = true;
        _;
        locked = false;
    }
}

Assessed type

Reentrancy

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

141345 marked the issue as duplicate of #51

@c4-pre-sort
Copy link

141345 marked the issue as duplicate of #1742

@c4-judge
Copy link

c4-judge commented Dec 8, 2023

alex-ppg marked the issue as satisfactory

@c4-judge c4-judge added satisfactory satisfies C4 submission criteria; eligible for awards partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%) and removed satisfactory satisfies C4 submission criteria; eligible for awards labels Dec 8, 2023
@c4-judge
Copy link

c4-judge commented Dec 8, 2023

alex-ppg marked the issue as partial-50

@c4-judge c4-judge added satisfactory satisfies C4 submission criteria; eligible for awards and removed partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%) labels Dec 8, 2023
@c4-judge
Copy link

c4-judge commented Dec 8, 2023

alex-ppg marked the issue as satisfactory

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-1517 satisfactory satisfies C4 submission criteria; eligible for awards
Projects
None yet
Development

No branches or pull requests

3 participants