Mint Manager and ERC721 Audit request

[zerongt9]

Audit request

Here is two contract related

1. ERC721-nftpass.sol , required a new parameter uint _level for storage the NFT level at smart contract
2. MintManager.sol , provide a public mint(erc20 payment) and private mint(no payment required) method to minting a nftpass NFT,also there are a mint period and quota to limit the number of minting.

Source code

https://github.com/technine-IT/live4well-smartcontract-for-audit

Payment plan

... Write [x] at the checkbox of the payment plan that suits your needs ...

• [x] Standard (standard projects such as ICOs, ERC20, ERC223, ERC721, ERC721A tokens and NFTs fall in this category - 2 auditors. $600 + $1.2 per line of code / minimum $1200)
• [x] Advanced (non-standard projects requiring more careful review - 3 auditors. $1200 + $1.5 per line / minimum $2400)
• [ ] Corporate (projects that require post-launch support - 3 auditors - Callisto Team will handle the bugbounty and provide a 1 month long period of technical support regarding the necessary security enhancement procedures and track/highlight any issues and potential threats - this plan allows to further apply for Callisto DAPP Insurance program)

Disclosure policy

... Do you want us to publish the report as it is or to notify you privately in case of finding critical mistakes? ...

... provide your conditions for publishing the report or leave only standard disclosure policy link ...

Standard disclosure policy.

Contact information (optional)

slack or email : zero.ng@technine.io

Platform

polygon

[yuriy77k]

@zerongt9 The audit fee is 2400 USDT. You may send USDT (ERC20 or BEP20) to: 0x6317c6944bd1cD3932d062cce39d7Fd602119529 (valid for Ethereum and Binance Smart Chain)

The estimated auditing time - is 7 days after payment.

[zerongt9]

@yuriy77k here is the payment tx hash 0xc40539d299ea0b68a9cbbd001e8d90f4b5fad85846253a2e084eb03f43b8a718

[yuriy77k]

@zerongt9 audit started

[zerongt9]

@yuriy77k are there any feedback / result?

[yuriy77k]

@zerongt9 The final report will be ready in 2 days.

[yuriy77k]

@zerongt9 We found a medium severity issue in the contract and the report was sent to your email.

[zerongt9]

we have fixed the issue and pushed to the same repo please help to arrange the re-audit

[yuriy77k]

Live4well ERC721 token and Mint Manager Security Audit Report

1. Summary

Live4well smart contract security audit report performed by Callisto Security Audit Department

This is re-audit of smart contracts that were fixed by developer according our recommendation.

2. In scope

Commit 467a0fcbc46b2cc94d9ae9ce63d6d565f318a2eb

• MintManager.sol
• ERC721-nftpass.sol
• IERC721-nftpass.sol

3. Findings

In total, 0 issues were reported, including:

• 0 high severity issues.

• 0 medium severity issues.

• 0 low severity issues.

In total, 12 notes were reported, including:

• 2 minor observations.

• 10 owner privileges.

3.1. Owner Privileges

Severity: owner privileges

Description

The ERC721-nftpass.sol and MintManager.sol contract inherits features of the OpenZeppelin AccessControl contract, allowing the administrator to manage administrators. This role can be abandoned, and it will result in blocking access to critical functions of the contract.

• NFTPass

  1. The safeMint function allows any user with MINT_ROLE permissions to mint an unlimited number of NFTs.
  2. The addMinter function allows the administrator to grant NFT minter role to any address.
  3. Function setBaseURI allows the owner to modify the baseURI_ of the NFT tokens.
  4. Function setTokenURI() allows owner to override the baseURI_ of NFT tokens to custom URI.

• MintManager

  1. The privateMint function, allows any address with PRIVATE_MINT rights, to min any number of NFTs for free, depending on the quota.
  2. The changeLevelPeriod function, allows the administrator to change the time range for a mint
  3. Function changeLevelquota, allows the administrator to change the quota for the NFT mint
  4. Function addNewERC20Price, allows the administrator to add any ERC20 tokens including poisoned tokens as payment for NFTs
  5. Function setPrivateMintRole, allows the administrator to give rights to any address to allow private minting.
  6. The transferERC20Token function allows the administrator to sign out any tokens from the contract.

Recommendation

Since the owner has unlimited rights to do everything, the ownership must be transferred to a multi-sig contract. And also the addresses to whom the rights PRIVATE_MINT, MINT_ROLE are given must be trusted.

3.2. The ERC-20 tokens price is set manually

Severity: minor observation

Description

The functions initializeNFTLevelInfo() and addNewERC20Price() are used by the owner to set the price of ERC-721 tokens in ERC-20 tokens. For non-stable coins, the value might fluctuate and the prices are required to be updated in case of a change in value. Allowing users to mint the ERC-721 token at an increased or decreased value depending on the price fluctuation.

Code Snippet

• https://github.com/technine-IT/live4well-smartcontract-for-audit/blob/467a0fcbc46b2cc94d9ae9ce63d6d565f318a2eb/smart-contract/MintManager.sol#L36
• https://github.com/technine-IT/live4well-smartcontract-for-audit/blob/467a0fcbc46b2cc94d9ae9ce63d6d565f318a2eb/smart-contract/MintManager.sol#L66

Recommendation

Consider using a price oracle to get the current value of the ERC-20 tokens and convert the value to a stablecoin to determine the amount of ERC-20 tokens required to mint an NFT by a user.

3.3. Follow good coding practice

Severity: minor observation

Description

1. Unlocked Pragma.

Contracts should be deployed using the same compiler version/flags with which they have been tested. Locking the floating pragma, i.e. by not using ^ in pragma solidity ^0.8.4, ensures that contracts do not accidentally get deployed using a compiler version with unfixed bugs.

2. Missing docstrings.

The contracts in the code base lack documentation. This hinders reviewers’ understanding of the code’s intention, which is fundamental to correctly assess not only security but also correctness. Additionally, docstrings improve readability and ease maintenance. They should explicitly explain the purpose or intention of the functions, the scenarios under which they can fail, the roles allowed to call them, the values returned, and the events emitted.

Consider thoroughly documenting all functions (and their parameters) that are part of the contracts’ public API. Functions implementing sensitive functionality, even if not public, should be documented as well. When writing docstrings, consider following the Ethereum Natural Specification Format (NatSpec).

3. Missing test suite.

The contract is missing a test suite to validate and verify the behavior of the contract functionalities. Add tests are recommended to ensure that the contract functions and behaves as expected.

4. Security practices

• [ ] Open-source contact.
• [ ] The contract should pass a bug bounty after the completion of the security audit.
• [ ] Public testing.
• [ ] Automated anomaly detection systems. - NOT IMPLEMENTED. A simple anomaly detection algorithm is recommended to be implemented to detect behavior that is atypical compared to normal for this contract. For instance, the contract must halt deposits in case a large amount is withdrawn in a short period until the owner or the community of the contract approves further operations.
• [ ] Multisig owner account.
• [ ] Standard ERC20-related issues. - NOT IMPLEMENTED. It is known that every contract can potentially receive an unintended ERC20-token deposit without the ability to reject it even if the contract is not intended to receive or hold tokens. As a result, it is recommended to implement a function that will allow extracting any arbitrary number of tokens from the contract.
• [ ] Crosschain address collisions. ETH, ETC, CLO, etc. It is possible that a transaction can be sent to the address of your contract at another chain (as a result of a user mistake or some software fault). It is recommended that you deploy a "mock contract" that would allow you to withdraw any tokens from that address or prevent any funds deposits. Note that you can reject transactions of native tokens deposited, but you can not reject the deposits of ERC20 tokens. You can use this source code as a mock contract: extractor contract source code. The address of a new contract deployed using CREATE (0xf0) opcode is assigned following this scheme keccak256(rlp([sender, nonce])). Therefore you need to use the same address that was originally used at the main chain to deploy the mock contract at a transaction with the nonce that matches that on the original chain. Example: If you have deployed your main contract with address 0x010101 at your 2021th transaction then you need to increase your nonce of 0x010101 address to 2020 at the chain where your mock contract will be deployed. Then you can deploy your mock contract with your 2021th transaction, and it will receive the same address as your mainnet contract.

5. Conclusion

The audited smart contract can be deployed. No security issues were found during the audit.

Users should pay attention to unlimited owner's rights.

It is recommended to adhere to the security practices described in pt. 4 of this report to ensure the contract's operability and prevent any issues that are not directly related to the code of this smart contract.

6. Previous audit report

https://github.com/CallistoSecurity/AuditReports/blob/main/Reports/Polygon_Live4wellERC721MintManager_report.md