Mintra Audit

Executive Summary

CoinFabrik was asked to audit the contracts for the Mintra project. The audited files are from the git repository .

Table of Contents

During this audit we found two critical issues, two medium issues and several minor issues. Also, several enhancements were proposed.

The two critical issues were resolved. One of the medium issues was resolved, the other one was partially resolved, and the rest of the issue can be considered as mitigated. All but one of the minor issues were resolved and the other one was partially resolved, and the rest of the issue can be considered as mitigated. Most of the enhancement proposals were implemented.

Scope

The scope for this audit includes and is limited to the following files:

contracts/buyAndBurn/BuyAndBurn.sol: The BuyAndBurn contract is intended to be used in conjunction with the FeeSplitter contract to use part of the generated fees to buy back MINT tokens and burn them,
contracts/feesplitter/FeeSplitter.sol: The FeeSplitter contract distributes fees generated from token transactions to three different addresses according to predefined percentages.
contracts/launchpad/ERC721Collection.sol: ERC721 token contract.
contracts/launchpad/IAdditionalFeatures.sol: Interface defining ERC721Collection settings.
contracts/launchpad/LaunchpadFactory.sol: Utility contract used to deploy ERC721Collection contracts.
contracts/marketplace/ERC1155Marketplace.sol: Marketplace implementation for ERC1155 tokens.
contracts/marketplace/ERC721Marketplace.sol: Marketplace implementation for ERC721 tokens.
contracts/marketplace/Marketplace.sol: The Marketplace contract is the base implementation for marketplaces, used in the ERC1155Marketplace and ERC721Marketplace contracts.
contracts/mint/MINT.sol: MINT token implementation.
contracts/mintStaking/interfaces/IMintStaking.sol: Interface definition for the MintStaking contract.
contracts/mintStaking/MintStaking.sol: The MintStaking contract implements a staking of an ERC20 token. The rewards are given in PLS.
contracts/msi/MSIFactory.sol: ERC721 token that wraps around the MintStaking contract.
contracts/msi/MSI.sol: Helper contract for MSIFactory.
contracts/msi/interfaces/IMSIFactory.sol: Interface for the MSIFactory contract.
contracts/msi/interfaces/IMSI.sol: Interface for the MSI contract.
contracts/msi/libraries/MintraSvgDivLib.sol: Utilities to generate SVG images. Used in the NFTDescriptor library.
contracts/msi/libraries/MintraSvgFilterLib.sol: Utilities to generate SVG images. Used in the NFTDescriptor library.
contracts/msi/libraries/MintraSvgGradientLib.sol: Utilities to generate SVG images. Used in the NFTDescriptor library.
contracts/msi/libraries/MintraSvgPatternLib.sol: Utilities to generate SVG images. Used in the NFTDescriptor library.
contracts/msi/libraries/NFTDescriptor.sol: The NFTDescriptor library is used to generate serverless token URIs in the MSIFactory contract.

No other files in this repository were audited. Its dependencies are assumed to work according to their documentation. Also, no tests were reviewed for this audit. In particular, it must be noted that the contracts/launchpad/ERC1155Collection.sol file is outside the scope of this audit.
The audit was performed in steps, expanding the scope along the way. On each step we expanded the scope of the files being reviewed, reaching the totality of the scope at the end of the process.

Step	Added directories	Commit hashes
1	/contracts/feeSplitter	3192b9e5244588f1a3ae131cb6e1acb9a05beee5
2	/contracts/marketplace	142b17d4c7f1db249a8385a0837df617fa08d270 4e74aed0f535db8d629db6e39548329a73de5214 ef7083307cca91f07bf8a97337aa8026ddb46972 d0e2b1de54ca6e7be96d98ada90cd97f0856a697
3	/contracts/buyAndBurn /contracts/mint	5ab13a28cacdf52661bc36f0c21d206aaab93973
4	/contracts/mintStaking /contracts/msi	ad95c867414a34aa78e2240db22e2006268962c3 47ae78fbc06bb6a0fc1b0e779037050aa12dfee0
5	/contracts/launchpad	24e7cb76a211af5c5ac40c37e46c8ae5d464bae7 98f842bdb4730cb72291616609c5d98d6826597b cb0f0beff8c5ca69012736a3fdd820a22f2bab48 0b998e9edbd0402938f296d13b7417d698aedf3348188721057fd632c5b324a5521eb5e672f4136a

The final status of all the issues and enhancements stated in this report corresponds to the latest commit in this table. All the listed commits were at the top of the main branch when we used them to conduct the audit.

Methodology

CoinFabrik was provided with the source code, including automated tests that define the expected behavior. Our auditors spent seven weeks auditing the source code provided, which includes understanding the context of use, analyzing the boundaries of the expected behavior of each contract and function, understanding the implementation by the development team (including dependencies beyond the scope to be audited) and identifying possible situations in which the code allows the caller to reach a state that exposes some vulnerability. Without being limited to them, the audit process included the following analyses:

Arithmetic errors
Outdated version of Solidity compiler
Race conditions
Reentrancy attacks
Misuse of block timestamps
Denial of service attacks
Excessive gas usage
Missing or misused function qualifiers
Needlessly complex code and contract interactions
Poor or nonexistent error handling
Insufficient validation of the input parameters
Incorrect handling of cryptographic signatures
Centralization and upgradeability

All the issues and enhancement proposals were communicated to the development team in the course of the audit and they provided the corresponding fixes. After each fix was provided we checked that the fix was properly applied and that no additional issues were introduced. This report includes the final status of all the reported issues and enhancement proposals.

Findings

In the following table we summarize the security issues we found in this audit. The severity classification criteria and the status meaning are explained below. This table does not include the enhancements we suggest to implement, which are described in a specific section after the security issues.

ID	Title	Severity	Status
CR-01	Steal All Rewards	Critical	Resolved
CR-02	All Rewards Locked in MSI	Critical	Resolved
ME-01	Marketplaces Payout Denial of Service	Medium	Partially Resolved
ME-02	Bidded Amount can be Reduced	Medium	Resolved
MI-01	FeeSplitter Transfers	Minor	Partially Resolved
MI-02	Zero Checks in FeeSplitter Constructor	Minor	Resolved
MI-03	Possible Reentrancy Attack on ERC1155Marketplace	Minor	Resolved
MI-04	Max Bid Increment Circumvention	Minor	Resolved
MI-05	BuyAndBurn Slippage-Check Circumvention	Minor	Resolved
MI-06	Unsafe Approval	Minor	Resolved
MI-07	Zero Checks in MSIFactory	Minor	Resolved
MI-08	Possible Reentrancy Attack on ERC721Collection	Minor	Resolved
MI-09	Royalty Settings not Respected	Minor	Resolved
MI-10	Missing Zero Check in LaunchpadFactory Constructor	Minor	Resolved

Severity Classification

Security risks are classified as follows:

Critical: These are issues that we manage to exploit. They compromise the system seriously. They must be fixed immediately.
Medium: These are potentially exploitable issues. Even though we did not manage to exploit them or their impact is not clear, they might represent a security risk in the near future. We suggest fixing them as soon as possible.
Minor: These issues represent problems that are relatively small or difficult to take advantage of, but might be exploited in combination with other issues. These kinds of issues do not block deployments in production environments. They should be taken into account and be fixed when possible.

Issues Status

An issue detected by this audit has one of the following statuses:

Unresolved: The issue has not been resolved.
Acknowledged: The issue remains in the code, but is a result of an intentional decision.
Resolved: Adjusted program implementation to eliminate the risk.
Partially resolved: Adjusted program implementation to eliminate part of the risk. The other part remains in the code, but is a result of an intentional decision.
Mitigated: Implemented actions to minimize the impact or likelihood of the risk.

Critical Severity Issues

CR-01 Steal All Rewards

Found on commit: ad95c867414a34aa78e2240db22e2006268962c3

Location:

contracts/mintStaking/MintStaking.sol: 156-162, 201-239

An attacker may steal all the PLS rewards handled by the MintStaking contract. In order to do so, it needs to stake some _stakeToken and then do a reentrancy attack on the harvest() function. Take notice that user.unclaimedReward is set to 0 in line 234 after sending PLS in line 223, making the attack possible.

It also needs to be stated that almost no funds are required, as the initial MINT to be used in the attack may be obtained via flash loan, as the stake-attack-unstake process can be made in a single transaction.

Recommendation

Mark the harvest() function as nonReentrant. Take notice that making updatePoolRewards() function to follow the checks-effects-interaction pattern is not enough. An attacker would be able to reenter into the unstake() function and steal some funds anyway.

Another option would be to rewrite the entire MintStaking contract to follow the checks-effects-interaction pattern and ditch the use of the nonReentrant modifier.

Status

Resolved. Fix checked on commit 47ae78fbc06bb6a0fc1b0e779037050aa12dfee0. The harvest() function was marked as nonReentrant.

CR-02 All Rewards Locked in MSI

Found on commit: 24e7cb76a211af5c5ac40c37e46c8ae5d464bae7

Location:

contracts/msi/MSI.sol: 69-78

When a new ERC721 token is minted via the MSIFactory contract a new instance of the MSI contract is created and the MINT tokens used to mint are transferred to this new contract instance (see the MSIFactory.mint() function for details). And then these funds are staked via the MSI.stake() function. After that the funds can only be recuperated via the MSI.unstake() function.

But this function checks for the awarded rewards before unstaking. This is the relevant code:

reward = address(this).balance;

// unstake token from MintStaking contract
mintStaking.unstake(stakedAmount);

// transfer unstaked MINT token to _recipient account
mintToken.safeTransfer(_recipient, user.userStakedMint);

// transfer PLS rewards back to _recipient account
(bool success, ) = _recipient.call{value: reward}("");

So all the rewards obtained by staking the MINT tokens are eternally blocked in this MSI instance.

Recommendation

Get the account balance after invoking the mintStaking.unstake() function.

Status

Resolved. Fix checked on commit 98f842bdb4730cb72291616609c5d98d6826597b. The contract balance is obtained after unstaking the tokens.

Medium Severity Issues

ME-01 Marketplaces Payout Denial of Service

Found on commit: 142b17d4c7f1db249a8385a0837df617fa08d270

Location:

contracts/marketplace/Marketplace.sol:277-286

The payout process, defined in the Marketplace.payout() function, can be interrupted by the actors that receive PLS in the process.

As we can see in contracts/marketplace/Marketplace.sol:277-286

// Send amount to seller
payable(_seller).transfer(sellerAmount);

// Send royalty amount to receiver

if (royaltyAmount > 0) {
   (bool success, ) = royaltyReceiver.call{value: royaltyAmount}("");
}

// Send market fee to fee splitter and distribute rewards

payable(feeSplitterAddress).transfer(marketFee);

the _seller, royalty Receiver and feeSplitterAddress receive PLS as part of the process. Any of those actors may cause a transaction reversion and abort the process.

It must be noted that doing a call() instead of a transfer() and ignoring the success flag is not enough, as the royaltyReceiver may enter in an infinite loop when receiving the PLS and exhaust all the available gas for the transaction.

The Marketplace.payout() function is called by the ERC721Marketplace.completeSale() and the ERC1155.completeSale() functions which are called by the ERC721Marketplace.buy(), ERC721Marketplace.endAuction(), ERC721Marketplace.acceptOffer(), ERC1155Marketplace.buy(), ERC1155Marketplace.endAuction() and ERC1155Marketplace.acceptOffer().

For the reasons explained above, this issue allows a rogue actor to stop the buy, auction ending and offer acceptance processes for both the ERC721Marketplace and the ERC1155Marketplace contracts.

Recommendation

Use the withdrawal pattern to transfer funds. Use call() instead of transfer() on each withdrawal operation and follow the checks-effect-interaction pattern or use a reentrancy guard. When using call() to transfer funds, remember to check that the invocation was successful. This recommendation is especially important when interacting with addresses not controlled by the deployer of the contract, such as _seller and royaltyReceiver.

Status

Partially resolved. Fix checked on commit ef7083307cca91f07bf8a97337aa8026ddb46972. The withdrawal pattern was implemented for the seller and the royalties owner. The transfer to the feeSplitterAddress is still in the code, so it may halt the payout procedure. It must be noted that the development team informed us that the feeSplitterAddress will point to the FeeSplitter contract, which is also being reviewed as part of this audit. It must also be noticed that the account that gets the funds must call the withdraw() function to effectively get the funds. This is specially important for contracts that are either sellers or royalties owners.

ME-02 Bidded Amount can be Reduced

Found on commit: 4e74aed0f535db8d629db6e39548329a73de5214

Location:

contracts/marketplace/ERC721Marketplace.sol:476
contracts/marketplace/ERC1155Marketplace.sol:546

By sending a smaller amount the offered item price can be lowered by a second buyer via the ERC721Marketplace.makeOffer() or ERC1155Marketplace.makeOffer() functions. This issue appears in the two marketplaces. This is the incorrect check (same code in the two functions):

msg.value >= (item.bidAmount * minBidIncrementPercent) / 100

Recommendation

msg.value should be greater or equal than item.bidAmount + (item.bidAmount * minBidIncrementPercent) / 100.

Status

Resolved. Fix checked on commit d0e2b1de54ca6e7be96d98ada90cd97f0856a697.

Minor Severity Issues

MI-01 FeeSplitter Transfers

Found on commit: 3192b9e5244588f1a3ae131cb6e1acb9a05beee5

Location:

contracts/feesplitter/FeeSplitter.sol:70-75

The logic used to split the fees between several contracts in the FeeSplitter.distributeRewards() function has several problems (shown below).

// update rewards for the staking contract
mintStakingContract.updateRewards{value: _stakingRewardsPls}();

// transfer tokens to the mintBurnContract for buyback and burn
payable(mintBurnContract).transfer(_buyBackAndBurnPls);

// transfer tokens to the root address
payable(rootAddress).transfer(_rootAddressPls);

These are:

Use of the transfer() function. This function may cause contracts to fail if the network changes the gas costs.
A faulty mintStakingContract, mintBurnContract or rootAddress may block the process, keeping the other 2 accounts without the funds that correspond to them.

This issue is considered minor as the 3 addresses mentioned are passed in the constructor of the contract, so they are not likely to be controlled directly by an attacker.

Recommendation

Status

Partially resolved. Fix checked on commit 4e74aed0f535db8d629db6e39548329a73de5214. The transfer() function is no longer used. Now it uses the call() function to transfer funds. The FeeSplitter.distributeRewards() function is now marked as non-reentrant, so reentrancy attacks are not possible. The withdrawal pattern was not applied, so a rogue mintStakingAddress, mintBurnContractAddress or rootAddress may still do a denial of service attack. This risk is mitigated given that the development team informed us that they control all those accounts.

MI-02 Zero Checks in FeeSplitter Constructor

Found on commit: 3192b9e5244588f1a3ae131cb6e1acb9a05beee5

Location:

contracts/feesplitter/FeeSplitter.sol:35-43

mintStakingContract, mintBurnContract or rootAddress may be set as the address 0 in the FeeSplitter constructor. This may lead to loss of funds.

Recommendation

Add require() statements checking that the addresses are not 0.

Status

Resolved. Fix checked on commit 4e74aed0f535db8d629db6e39548329a73de5214. Zero checks were added.

MI-03 Possible Reentrancy Attack on ERC1155Marketplace

Found on commit: 4e74aed0f535db8d629db6e39548329a73de5214

Location:

contracts/marketplace/ERC1155Marketplace.sol:528-583

The ERC1155Marketplace.makeOffer() function does not follow the check-effects-interaction pattern, nor is marked as nonReentrant, nor any proof is offered that it is immune to reentrancy attacks, so a possible reentrancy attack cannot be discarded. As part of the audit we tried to find an attack without success, so this issue is considered minor.

Recommendation

Either mark the function as nonReentrant, refactor it to follow the checks-effects-interaction pattern or provide a proof that it cannot be used to perform a reentrancy attack.

Status

Resolved. Fix checked on commit d0e2b1de54ca6e7be96d98ada90cd97f0856a697. The function was marked as nonReentrant.

MI-04 Max Bid Increment Circumvention

Found on commit: 4e74aed0f535db8d629db6e39548329a73de5214

Location:

contracts/marketplace/ERC721Marketplace.sol: 199-206
contracts/marketplace/ERC1155Marketplace.sol: 234-242
contracts/marketplace/Marketplace.sol: 174-177

A user may circumvent the max bid increment percentage check, included in the Marketplace.checkBidValidity modifier, by invoking several times the ERC721Marketplace.createBid() or the ERC1155Marketplace.createBid() functions.

It must be noted that checking that the same account does not invoke the functions twice in a row is not enough, as an attacker may choose to use different accounts to do so.

Recommendation

Remove the max bid increment check in the Marketplace.checkBidValidity modifier, and all the code to set the configuration.

Status

Resolved. Fix checked on commit 5ab13a28cacdf52661bc36f0c21d206aaab93973.

MI-05 BuyAndBurn Slippage-Check Circumvention

Found on commit: 5ab13a28cacdf52661bc36f0c21d206aaab93973

Location:

contracts/buyAndBurn/BuyAndBurn.sol:119-175

An attacker may call the BuyAndBurn.buybackAndBurn() function several times to circumvent the slippage check. If the slippage check is avoided, then the buyback procedure can be used to manipulate the price of the MINT token price (pointed by the _mintTokenAddress in the source code) in the DEX at plsxRouter, This, in turn, allows an attacker to do a sandwich attack on the MINT token price extracting value from the price difference.

This attack vector is already mitigated by allowing only EOAs to initiate the buyback procedure but if the transaction order is controlled by an attacker via gas price or collusion with a validator or miner it can still be made.

Recommendation

Do not allow the buyback procedure to be executed twice in the same block, or in 2 consecutive blocks, allowing the rest of the participants of the blockchain to operate at the intermediate price.

Status

Resolved. Fix checked on commit ad95c867414a34aa78e2240db22e2006268962c3. Now the buyback procedure can be executed only once per block. Allowing the rest of the participants of the system to operate at the intermediate prices.

MI-06 Unsafe Approval

Found on commit: 47ae78fbc06bb6a0fc1b0e779037050aa12dfee0

Location:

contracts/msi/MSI.sol:53

When using OpenZeppelin’s SafeERC20 library it is a bad practice to set an allowance to an absolute non-zero value. It is recommended to use SafeERC20.safeIncreaseAllowance() and SafeERC20.safeDecreaseAllowance() instead.

Recommendation

Use SafeERC20.safeIncreaseAllowance() to give the allowance.

Status

Resolved. Fix checked on commit 24e7cb76a211af5c5ac40c37e46c8ae5d464bae7.

MI-07 Zero Checks in MSIFactory

Found on commit: 24e7cb76a211af5c5ac40c37e46c8ae5d464bae7

Location:

contracts/msi/MSIFactory.sol:32-38

mintStaking or mintToken may be set as the address 0 in the FeeSplitter constructor. This may lead to undefined behavior.

Recommendation

Add require() statements checking that the addresses are not 0.

Status

Resolved. Fix checked on commit 98f842bdb4730cb72291616609c5d98d6826597b. The require statements were added.

MI-08 Possible Reentrancy Attack on ERC721Collection

Found on commit: 24e7cb76a211af5c5ac40c37e46c8ae5d464bae7

Location:

contracts/launchpad/ERC721Collection.sol: 122-152

The ERC721Collection.mint() function could be exploited with a reentrancy attack, bypassing the checks for the maximum number of tokens (contracts/launchpad/ERC721Collection.sol:126) and maximum number of token for the account (contracts/launchpad/ERC721Collection.sol:127-130) by calling again the function when the fee splitter receives the funds (contracts/launchpad/ERC721Collection.sol:146). This attack may be triggered by any contract in the fee splitter call graph.

The severity of this issue has been lowered given that the fee splitter address is controlled by the deployer of the contract and the way contracts are expected to be deployed (see Analyzed Contract Dependencies).

Recommendation

The ERC721Collection.mint() function cannot be written using the checks-effects-interaction pattern because both the transfer in line 146 and the call to _safeMint() function invoked in line 151 may potentially be used to invoke code used to trigger a reentrancy attack. So we recommend marking the mint() function as nonReentrant.

Status

Resolved. Fix checked on commit cb0f0beff8c5ca69012736a3fdd820a22f2bab48. The ERC721Collection.mint() function was marked as nonReentrant.

MI-09 Royalty Settings not Respected

Found on commit: 24e7cb76a211af5c5ac40c37e46c8ae5d464bae7

Location:

contracts/launchpad/ERC721Collection.sol

The ERC721Collection contract does not respect the additionalFeatures.royaltyOperator nor additionalFeatures.royaltyFee settings for royalties.

Recommendation

Either use these values to set the default royalty settings in the constructor or remove the settings.

Status

Resolved. Fix checked on commit cb0f0beff8c5ca69012736a3fdd820a22f2bab48. Default royalty settings are set if the royaltyOperator set in the _additionalFeatures constructor parameters is not zero.

MI-10 Missing Zero Check in LaunchpadFactory Constructor

Found on commit: cb0f0beff8c5ca69012736a3fdd820a22f2bab48

Location:

contracts/launchpad/LaunchpadFactory.sol: 52-69

While _feeSplitterAddress is controlled for the zero address in the LaunchpadFactory constructor, the _robinHood parameter is not. If the _robinHood is zero, then the market percent cannot be decreased via the LaunchpadFactory.setMarketPercent() function.

Recommendation

Add a require statement checking that the _robinHood address is not zero.

Status

Resolved. Fix checked on commit 48188721057fd632c5b324a5521eb5e672f4136a. The require statement was added.

Enhancements

These items do not represent a security risk. They are best practices that we suggest implementing.

ID	Title	Status
EN-01	Marketplace Analytics Should be off Chain	Implemented
EN-02	MINT Stats Should be off Chain	Implemented
EN-03	BuyAndBurn Stats Should be off Chain	Implemented
EN-04	Configurable Bounty For BuyAndBurn	Not implemented
EN-05	MSIFactory Limitations	Implemented
EN-06	Wei Lost in MintStaking	Not implemented
EN-07	LaunchpadFactory ERC1155 Support	Implemented

EN-01 Marketplace Analytics Should be off Chain

Suggested on commit: 4e74aed0f535db8d629db6e39548329a73de5214

Location:

contracts/marketplace/Marketplace.sol: 120-123,516-608

If the statistics collected using the tokenAnalytics, collectionAnalytics and globalAnalytics variables in the Marketplace contract are not being used on chain by another contract it is better to emit events for all the actions in the marketplace and collect them off chain, as it would simplify the contract code and reduce gas costs.

Recommendation

Remove the tokenAnalytics, collectionAnalytics and globalAnalytics variables in the Marketplace contract.
Either:
1. Change the updateCollectionAnalytics(), updateItemAnalytics(), updateAnalytics() functions in the Marketplace contract to emit events with all the necessary information to collect and process the analytics off chain.
2. Remove those functions and its invocations and emit the events directly in the code.

Status

Implemented. Checked on commit 5ab13a28cacdf52661bc36f0c21d206aaab93973. All the analytics code was removed.

EN-02 MINT Stats Should be off Chain

Suggested on commit: 5ab13a28cacdf52661bc36f0c21d206aaab93973

Location:

contracts/mint/MINT.sol: 7, 28, 55-57

If the total amount of MINT tokens burned is not being used on chain by another contract, there is no need to spend gas to do the tracking.

Recommendation

Remove the _totalBurned variable, its uses and the totalBurned() function. Also notice that the amount of tokens burned can be calculated by looking at the Transfer() events that are already being emitted in the OpenZeppelin’s ERC20 contract, that is the parent of the MINT contract.

Status

Implemented. Checked on commit ad95c867414a34aa78e2240db22e2006268962c3.

EN-03 BuyAndBurn Stats Should be off Chain

Suggested on commit: 5ab13a28cacdf52661bc36f0c21d206aaab93973

Location:

contracts/buyAndBurn/BuyAndBurn.sol: 48-50, 213-251

If the values tracked in the startDay, lastDay and dailyBurnSnapshot variables in the BuyAndBurn contract are not being used by another contract on chain, there is no need to spend the gas to keep this state.

Recommendation

Remove the startDay, lastDay and dailyBurnSnapshot variables
Remove the getTotalSupplySnapshots(), _recordTotalSupplySnapshot() and _recordMINTBurnData() functions and its invocations.

Status

Implemented. Checked on commit ad95c867414a34aa78e2240db22e2006268962c3.

EN-04 Configurable Bounty For BuyAndBurn

Suggested on commit: 5ab13a28cacdf52661bc36f0c21d206aaab93973

Location:

contracts/buyAndBurn/BuyAndBurn.sol: 132

If the price of the MINT token or PLS moves wildly the reward for the buyback procedure may not be enough to do the procedure or too much, wasting valuable PLS that can be used to burn MINT token instead.

Recommendation

Make the bounty percentage for the buyback procedure configurable by the vulcan in the BuyAndBurn contract.

Status

Not implemented. The development team informed us that they will not implement this suggestion.

EN-05 MSIFactory Limitations

Suggested on commit: 47ae78fbc06bb6a0fc1b0e779037050aa12dfee0

Location:

contracts/msi/MSI.sol
contracts/msi/MSIFactory.sol

The MintStaking contract allows for the rewards obtained via the staked tokens to be harvested by the staker via the MintStaking.harvest(). But the MSIFactory and MSI contracts do not have the functionality to do this harvesting. It must be noted that it is stated in the code comments that this functionality is available (see contracts/msi/MSI.sol:17).

Recommendation

Either implement the missing functionality or document its absence.

Status

Implemented. As this is the expected behavior, the documentation was changed to reflect it.

EN-06 Wei Lost in MintStaking

Suggested on commit: 47ae78fbc06bb6a0fc1b0e779037050aa12dfee0

Location:

contracts/mintStaking/MintStaking.sol

When divisions are made in integer arithmetic, a non-zero remainder may occur. Because of it the arithmetic calculations used to give out rewards in the MintStaking contract will leave some wei trapped in it. This wei will never be awarded as a reward with the current logic.

Recommendation

Reimplement the calculations made to award the rewards in a way that all the available rewards are distributed.

Status

Not implemented. This problem has been acknowledged by the development team. They informed us that they do not plan to address this.

EN-07 LaunchpadFactory ERC1155 Support

Suggested on commit: cb0f0beff8c5ca69012736a3fdd820a22f2bab48

Location:

contracts/launchpad/LaunchpadFactory.sol

While the LaunchpadFactory comments claim that both ERC721 and ERC1155 token collections can be launched using the contract, only the ERC721 collections are supported.

Recommendation

Either add the ERC1155 collection support or change the documentation and the code to reflect that only ERC721 collections are supported. If the second path is taken, all mentions of ERC1155 should be removed.

Status

Implemented. Checked on commit 0b998e9edbd0402938f296d13b7417d698aedf33. All references to ERC1155 were removed.

Other Considerations

The considerations stated in this section are not right or wrong. We do not suggest any action to fix them. But we consider that they may be of interest to other stakeholders of the project, including users of the audited contracts, token holders or project investors.

Libraries

Three different npm libraries were used in the implementation of the analyzed smart contracts:

@openzeppelin/contracts is used all over the code.
erc721a is used to implement two ERC721-token contracts, MSIFactory and ERC721Collection.
base64-sol is used to generate the token uri in the NFTDescriptor library.

Those libraries are assumed to work as documented.

Blockchain

The analyzed contracts are developed to be deployed in pulsechain. See https://pulsechain.com/ for more information on this blockchain. Pulsechain is an ethereum-compatible blockchain and its native token is named PLS.

Analyzed Contract Dependencies

The analyzed contracts are designed to be deployed together. The way contracts are expected to be deployed is documented in the scripts/allContracts/deployAll.ts script.
From this file these dependencies are deduced (divided in 2 diagrams to make it easier to comprehend):

The boxes represent each deployed contract and a single EOA (vulcan). The arrows show how these accounts are passed in the constructors of other contracts. The label in the arrow corresponds to the name of the associated constructor parameter.
This audit assumes that the contract’s deployment will respect this.

Analyzed Contract Dependencies

An item represents an item to be sold in any of the marketplaces. In the ERC721Marketplace contract is identified by the (contract address, token id) pair of the underlying token. But in the ERC1155Marketplace the (contract address, token id, seller address) triplet is used given that the same token may have multiple instances.

There are 3 types of items:

Offer: It means that someone offered to buy a token not being sold in the store.
Auction: The owner of the token sells it via an auction handled by the marketplace.
Sale: The owner of the tokens sells it at a predetermined price.

It must be noted that an ERC1155 offer item is also identified by a seller address, so ERC1155 offers are for a single seller.

This state diagram shows the possible item states, and applies to both the ERC721Marketplace and the ERC1155Marketplace contracts. The edges are noted with the names of the external or public functions used to change the item state.

Marketplace Royalties

The Marketplace contract gives royalties when operations are performed on tokens.

If the token implements ERC2981, checked by probing the token’s royaltyInfo() function, then royalties are sent to the royalty receiver according to the declaration made by the token contract.

If the token does not implement ERC2981, then the token needs to implement the owner() function, and royalties are distributed according to the settings made via the Marketplace.createOrUpdateRoyality() function to the address returned by the owner() function.

If neither of those conditions is respected, then no royalties are distributed.

It must be noted that royalties can be increased and exceed the maximum royalty percentage (stored in the Marketplace.maxRoyaltyBasisPoints variable) for ERC2981 tokens but not for other tokens.

MintStaking Behavior

The expected behavior of the MintStaking contract is documented in the doc/Staking.txt file. This documentation addition was requested during the course of this audit.

Upgrades

There are no mechanisms to upgrade the audited contracts.

Centralization

While an effort was made to make the analyzed contracts decentralized, there are several roles that have centralized power, such as ERC721Collection owner, LaunchpadFactory robinhood and the BuyAndBurn and marketplaces vulcan. All those roles are awarded to a single address. See the Privileged Roles section for more details.

There are no admin-like roles in any of the audited contracts with the capabilities to upgrade the deployed code nor stop the contracts functionality and withdraw the contract funds. This is a double edged sword. While as a user of the contracts we can be sure that the code will not be changed by any malicious attacker or administrator, if a critical bug is found after the deployment of any of the audited contracts there is no way an administrator can rescue the funds locked in the contracts nor stop users from using the contracts.

Privileged Roles

These are the privileged roles that we identified on each of the audited contracts.

BuyAndBurn

Vulcan

The vulcan can change the maximum allowed price slippage generated when running the buybackAndBurn() function. Its value can be set between 0.1% and 10%.

The account associated with this role is set in the constructor contract and cannot be changed afterwards.

EOA

Only an EOA can execute the buyBackAndBurn() function.

ERC721Collection

Owner

The owner can:

set the base uri for the token collection via the setBaseUri() function.
mint tokens without paying for them via the devMint() function.
withdraw all the funds received when new tokens are minted via the withdraw() function.
set the default receiver and fees for royalties via the setDefaultRoyalty() function.
set a new owner via the transferOwnership() function (defined in Ownable).
make the contract ownerless via the renounceOwnership() function (defined in Ownable).

By default the owner of the contract is the deployer. If deployed via the LaunchpadFactory contract the initial owner is set to the caller of the LaunchpadFactory.deployCollection() function.

LaunchpadFactory

Robinhood

The Robinhood can decrease the market percent via the setMarketPercent() function.

The account associated with this role is set in the constructor contract and cannot be changed afterwards

Marketplace

Vulcan

The vulcan can:

Set the maximum bid increment percentage via the setMaxBidIncrementPercent() function. This value must be between 50% and 150%. The initial value is 100%.
Set the minimum bid increment percentage via the setMinBidIncrementPercent() function. This value must be between 5% and 15%. The initial value is 10%.
Set the minimum auction length via the setMinAuctionLength() function. This value must be between 1 hour and 24 hours.
Set the maximum auction length via the setMaxAuctionLength() function. This value must be between 30 days and 364 days.
Set the floor price via the setFloorPrice() function. It must be between 1 PLS and 1000 PLS.
Set the market fee percentage via the setMarketPercent() function. It cannot be lower than 1% and it must be lower than the older value. The initial value is 2.25%.

The account associated with this role is set in the constructor contract and cannot be changed afterwards.

It must be noted that the check for this role is made via the onlyOwner() modifier.

MSI

Factory

Only the factory can change state via functions in the MSI contract. The only functions available to change state are stake() and unstake().

The account associated with this role is set in the constructor contract and cannot be changed afterwards.

Changelog

2023-04-19 – Report MI-01 and MI-02.
2023-04-24 – Report ME-01.
2023-04-26 – Check fix for ME-01, MI-01, MI-02. Report ME-02, MI-03, MI-04.
2023-04-28 – Check fix for ME-02, MI-03. Suggest EN-01.
2023-05-04 – Check fix for MI-04. Check implementation for EN-01. Suggest EN-02, EN-03.
2023-05-08 – Report MI-05. Suggest EN-04.
2023-05-10 – Report CR-01. Check fix for MI-05. Check implementation for EN-02. EN-03. Document EN-04.
2023-05-16 – Check fix for CR-01. Suggest EN-05, EN-06. Report MI-06.
2023-05-19 – Report CR-02, MI-07. Document EN-06. Check fix for MI-06. Check implementation for EN-05.
2023-05-24 – Report MI-08, MI-09. Check fix for CR-02, MI-07.
2023-05-31 – Check fixes for MI-08, MI-09. Suggest EN-07.
2023-06-01 – Report MI-10. Check implementation for EN-07.
2023-06-02 – Check fix for MI-10. Add Executive Summary, Scope, Methodology, and Other Considerations sections.

Disclaimer: This audit report is not a security warranty, investment advice, or an approval of the Mintra project since CoinFabrik has not reviewed its platform. Moreover, it does not provide a smart contract code faultlessness guarantee.