DancingSeaHorse - Audit

CertiK9 Sep 2022Files named in the report

CertiK reviewed DancingSeaHorse (DancingSeaHorse - Audit) on 9 Sep 2022: 16 findings, 0 critical; 7 marked fixed, 1 acknowledged, 8 open.

No open critical findings

Open the report Open DancingSeaHorse’s security record

Report hosted on skynet.certik.com.

Trustblock does not review code.

Findings16

Statuses are the security firm’s own, from its report.

Severity	Open	Acknowledged	Fixed
Critical	0	0	0
High	1	0	1
Medium	0	1	0
Low	7	0	6

HighPotential Reentrancy AttackFixed

The `_safeMint()` in ERC721 contract contains an `_checkOnERC721Received()` function, if the `to` address is a smart contract, a reentrancy attack can occur when `_checkOnERC721Received()` makes an external call to this untrusted contract before resolving any effects. If the attacker can control the untrusted contract, they can make a recursive call back to the original function, repeating interactions that would have otherwise not run after the external call resolved the effects. e.g. The attackers could use the attacking contract to call `premintNFT()` as many times as they want, and the `premintSaleCounter < premintlimit` check will always be `true` for the `premintSaleCounter` would only be added once at the end of the function. Besides, if `premintNFT()` is called while the `for` loop in `makeNft` is in progress, the `tokenid` for the next round of `for` loop will be occupied, causing the function to revert.

Low`msg.value` should be equal to the `cost`Fixed

The `msg.value` should equal to the `cost * _mintAmount` to prevent investors transferring excess ETHs to the contract.

LowLogical issue in the `maxSupply` checkFixed

The `maxSupply` check should be changed from `supply <= maxSupply` to `supply + _mintAmount <= maxSupply` to guarantee the `maxSupply` will not be exceeded.

LowMissing Zero Address ValidationFixed

Addresses should be checked before assignment or external call to make sure they are not zero addresses. ```solidity=156 tokenAddress = _tokenAddress; ``` - `_tokenAddress` is not zero-checked before being used.

LowRedundant codesFixed

The function `giftNft()`, `OTC()`, and `vault()` seem exactly the same, hence two of them are redundant.

LowThe use of `withdrawAll()`Fixed

The function `withdrawAll()` is used to withdraw a fixed amount of BNBs from the contract, which is the balance of `t1` address. However, the function would be reverted if the balance of `t1` is large and the BNBs in the contract are not enough, while in the other case it may take many times to withdraw all the BNBs in the contract.

LowUnnecessary Use of SafeMathFixed

The `SafeMath` library is used unnecessarily. With Solidity compiler versions 0.8.0 or newer, arithmetic operations will automatically revert in case of integer overflow or underflow. ```solidity=14 using SafeMath for uint256; ``` - `SafeMath` library is used for `uint256` type in `Nft` contract. --- ```solidity=14 using SafeMath for uint256; ``` - `SafeMath` library is used for `uint256` type in `Nft` contract. ```solidity=151 addBrandCount(count.add(_mintAmount)); ``` - `SafeMath.add` is called in `brandNft` function of `Nft` contract. *Note: Only a sample of 2 `SafeMath` library usage in this contract (out of 5) are shown above.*