User can overpay for NFT

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

Because of the way the check require(msg.value >= getPrice(_mintCollectionID), "Wrong ETH"); is structured, users can end up paying more than the actual value of NFT, causing bad user experience for user. There is also no refund for extra value sent to help the UX.

Proof of Concept

In the burnToMint function as shown below

function burnToMint(uint256 _burnCollectionID, uint256 _tokenId, uint256 _mintCollectionID, uint256 _saltfun_o) public payable {
        require(burnToMintCollections[_burnCollectionID][_mintCollectionID] == true, "Initialize burn");
        require(block.timestamp >= collectionPhases[_mintCollectionID].publicStartTime && block.timestamp<=collectionPhases[_mintCollectionID].publicEndTime,"No minting");
        require ((_tokenId >= gencore.viewTokensIndexMin(_burnCollectionID)) && (_tokenId <= gencore.viewTokensIndexMax(_burnCollectionID)), "col/token id error");
        // minting new token
        uint256 collectionTokenMintIndex;
        collectionTokenMintIndex = gencore.viewTokensIndexMin(_mintCollectionID) + gencore.viewCirSupply(_mintCollectionID);
        require(collectionTokenMintIndex <= gencore.viewTokensIndexMax(_mintCollectionID), "No supply");
        //@audit-check: avoid overpaying for nft: use ==: duplicate medium
        require(msg.value >= getPrice(_mintCollectionID), "Wrong ETH");
        uint256 mintIndex = gencore.viewTokensIndexMin(_mintCollectionID) + gencore.viewCirSupply(_mintCollectionID);
        // burn and mint token
        address burner = msg.sender;
        gencore.burnToMint(mintIndex, _burnCollectionID, _tokenId, _mintCollectionID, _saltfun_o, burner);
        collectionTotalAmount[_mintCollectionID] = collectionTotalAmount[_mintCollectionID] + msg.value;
    }

The price check does not put an upper bound, means User can pay 1000ETH for a 1ETH NFT with no refund sent

Tools Used

Manual Review

Recommended Mitigation Steps

• use require(msg.value == getPrice(_mintCollectionID), "Wrong ETH"); instead or
• add a refund function for excess ETH sent

Assessed type

ETH-Transfer

[c4-pre-sort]

141345 marked the issue as duplicate of #304

[c4-judge]

alex-ppg marked the issue as not a duplicate

[c4-judge]

alex-ppg marked the issue as primary issue

[alex-ppg]

The Warden specifies that excess funds sent during a purchase operation for an NFT are not refunded which directly correlates to issue M-02 of the bot report which has been aptly marked as Medium.

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Out of scope

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Out of scope