Mae Drop1 Contracts - TAEX

The price of an NFT can change and there is no slippage to protect against it, causing potential loss for users.

Upon public and whitelist minting, users are charged an amount of funds based on the amount of NFTs minted by them:

uint256 totalCost = publicPrice * _quantity;
require(msg.value >= totalCost, "Insufficient payment");

If the user has provided more than supposed to, he gets refunded the excess:

if (msg.value > totalCost) {
      (bool success, ) = payable(msg.sender).call{ value: msg.value - totalCost }("");
      require(success, "Refund failed");
}

The issue is that the price can change at any moment as there is a setter function for it:

function setPricing(uint256 _whitelistPrice, uint256 _publicPrice) external onlyModerator {
   whitelistPrice = _whitelistPrice;
   publicPrice = _publicPrice;
   emit PricingUpdated(_whitelistPrice, _publicPrice);
}

This allows the following scenario to occur:

1. User provides all of his 9 ETH upon a public mint, expecting to get refunded the excess.

2. He sees the price is 2 ETH per NFT and decides to mint 4 NFTs, a total price of 8 ETH.

3. A moderator changes the price to 2.25 ETH due to the huge demand at a similar time as the user's public mint.

4. The moderator's transaction executes first, resulting in the user to be charged all of his 9 ETH.

Consider allowing users to provide a slippage input.

RISK ACCEPTED: The TAEX team has accepted the risk of this finding.

The block chain ID is not included in the message hash which allows a cross-chain signature replay under specific conditions.

Upon using the Wert minting functionality, the message hash is generated as follows without including the chain ID. It is then validated against the Wert partner.

A cross-chain signature replay is possible when the below 2 conditions are met as the block chain ID is not included in the message hash:

1. The contract has the same deployment address on all chains as the contract address is included in the message hash.

2. The wert partner address is the same on all chains.

If the conditions mentioned in the report are likely to occur in your specific context, then consider including the block chain ID in the message hash.

RISK ACCEPTED: The TAEX team has accepted the risk of this finding.

The current phase initialization is redundant as it is set to the default value of the enum.

The current phase gets set to paused in the initializer function. However, this is unnecessary as the paused phase is the default value in the enum.

As the paused phase is default one, then the code is simply overwriting the current phase with the value it had initially.

Consider removing the line discussed in the report.

ACKNOWLEDGED: The TAEX team has acknowledged this finding.

Whenever an NFT is minted, its metadata gets initialized as follows:

tokenMetadata[tokenId] = TokenMetadata({ isRevealed: false, revealTimestamp: 0 });

However, this is completely unnecessary as both of the initialized values are the default values.

Consider removing the initialization of token metadata.

ACKNOWLEDGED: The TAEX team has acknowledged this finding.

Multiple places in the code conduct the following cast to a payable address. However, this cast is completely unnecessary as the CALL opcode does not require the address being called to be payable.

Consider removing the payable casts.

ACKNOWLEDGED: The TAEX team has acknowledged this finding.

The contract in scope currently uses floating pragma versions ^0.8.22 which means that the code can be compiled by any compiler version that is greater than or equal to 0.8.22, and less than 0.9.0.

However, it is recommended that contracts should be deployed with the same compiler version and flags used during development and testing. Locking the pragma helps to ensure that contracts do not accidentally get deployed using another pragma. For example, an outdated pragma version might introduce bugs that affect the contract system negatively.

In this aspect, it is crucial to select the appropriate EVM version when it's intended to deploy the contracts on networks other than the Ethereum mainnet, which may not support these opcodes. Failure to do so could lead to unsuccessful contract deployments or transaction execution issues.

Lock the pragma version to the same version used during development and testing. Additionally, make sure to specify the target EVM version when using Solidity versions from 0.8.20 and above if deploying to chains that may not support newly introduced opcodes. Additionally, it is crucial to stay informed about the opcode support of different chains to ensure smooth deployment and compatibility.

ACKNOWLEDGED: The TAEX team has acknowledged this finding.

In Solidity smart contract development, replacing hard-coded revert message strings with the Error() syntax is an optimization strategy that can significantly reduce gas costs. Hard-coded strings, stored on the blockchain, increase the size and cost of deploying and executing contracts.

The Error() syntax allows for the definition of reusable, parameterized custom errors, leading to a more efficient use of storage and reduced gas consumption. This approach not only optimizes gas usage during deployment and interaction with the contract but also enhances code maintainability and readability by providing clearer, context-specific error information.

It is recommended to replace hard-coded revert strings in require statements for custom errors, which can be done following the logic below:

1. Standard require statement (to be replaced):

require(condition, "Condition not met");

2. Declare the error definition to state

error ConditionNotMet();

3. As currently is not possible to use custom errors in combination with require statements, the standard syntax is:

if (!condition) revert ConditionNotMet();

More information about this topic in the Official Solidity Documentation.

ACKNOWLEDGED: The TAEX team has acknowledged this finding.

The project is using Solidity version ^0.8.22 which supports named mappings. Using named mappings can improve the readability and maintainability of the code by making the purpose of each mapping clearer. This practice helps developers and auditors understand the mappings' intent more easily.

Consider refactoring the mappings to use named arguments, which will enhance code readability and make the purpose of each mapping more explicit.

For example, in the MAENFTCollection.sol contract, instead of declaring:

mapping(address => uint256) public addressMintCount;

It could be declared as:

mapping(address user => uint256 mintCount) public addressMintCount;

ACKNOWLEDGED: The TAEX team has acknowledged this finding.

The addToWhitelist() , removeFromWhitelist() , and setMoonPhase() functions are responsible for changing important contract state, however, they don't emit events.

Modify the addToWhitelist() , removeFromWhitelist() , and setMoonPhase() functions, so that they emit proper events when called.

ACKNOWLEDGED: The TAEX team has acknowledged this finding.