Mini Miners - Seascape

In the MinerGame contract the exportNft() function is used to "unstake" the Mine NFT:

MinerGame.sol

function exportNft(uint256 _nftId) external {
  require(mineOwners[_nftId] == msg.sender, "MinerGame: Not the owner");

  MineNFT nft = MineNFT(mineNft);
  nft.safeTransferFrom(address(this), msg.sender, _nftId);

  PlayerParams storage _player = player[msg.sender];
  delete _player.nftId;
  delete _player.stakeTime;

  delete mineOwners[_nftId];

  emit ExportNft(msg.sender, _nftId, block.timestamp);        
}

\color{black} \color{white}

As we can see above, the Mine NFT is sent back to the user with a safeTransferFrom() call. This safeTransferFrom() calls check if the receiver is a smart contract and if so, it calls the _checkOnERC721Received hook.

This passed the control flow to the receiver and opens up a reentrancy vulnerability as the user, in this case the smart contract, got the NFT but the state variables player[msg.sender] and mineOwners[_nftId] are still not deleted/updated.

In the MinerGame contract the goldChangeToken() creates an storage pointer to the player mapping but then it does not make any use of it:

MinerGame.sol

function goldChangeToken(uint256 _gold, uint8 _v, bytes32 _r, bytes32 _s) external {
  require(_gold > 0, "MinerGame: The exchange amount must greater than zero");

  uint256 chainId;   
  assembly {
      chainId := chainid()
  }

  PlayerParams storage _player = player[msg.sender];

  {
    bytes memory prefix     = "\x19Ethereum Signed Message:\n32";
    bytes32 message         = keccak256(abi.encodePacked(_gold, msg.sender, nonce, address(this), chainId));
    bytes32 hash            = keccak256(abi.encodePacked(prefix, message));
    address recover         = ecrecover(hash, _v, _r, _s);

    require(recover == verifier, "Verification failed about stakeToken");
  }

  nonce++;

  uint256 _tokenAmount = _gold * MULTIPLIER / ratio;
  _safeTransfer(token[0], msg.sender, _tokenAmount);

  emit GoldChangeToken(msg.sender, _gold, _tokenAmount, block.timestamp);  

}

\color{black} \color{white}

In multiple contracts there are functions marked as public but they are never directly called within the same contract or in any of their descendants:

MineNFTFactory.sol

• mint() (MineNFTFactory.sol#34-37)
• setNft() (MineNFTFactory.sol#42-44)
• addAdmin() (MineNFTFactory.sol#47-50)
• renounceAdmin() (MineNFTFactory.sol#53-56)
• addGenerator() (MineNFTFactory.sol#85-88)
• removeGenerator() (MineNFTFactory.sol#91-94)

MineNFT.sol

• mint() (MineNFT.sol#35-46)
• setOwner() (MineNFT.sol#48-50)
• setFactory() (MineNFT.sol#52-54)
• setBaseUri() (MineNFT.sol#56-58)

MinerGame.sol

• withdraw() (MinerGame.sol#188-195)
• addToken() (MinerGame.sol#198-206)
• setScale() (MinerGame.sol#209-212)

CrownsToken.sol

• burn() (CrownsToken.sol#96-98)
• burnFrom() (CrownsToken.sol#111-118)
• name() (CrownsToken.sol#127-129)
• symbol() (CrownsToken.sol#135-137)
• decimals() (CrownsToken.sol#148-150)
• totalSupply() (CrownsToken.sol#155-157)
• balanceOf() (CrownsToken.sol#162-164)
• transfer() (CrownsToken.sol#173-176)
• approve() (CrownsToken.sol#203-206)
• transferFrom() (CrownsToken.sol#217-227)
• increaseAllowance() (CrownsToken.sol#241-244)
• decreaseAllowance() (CrownsToken.sol#260-268)

MscpToken.sol

• burn() (MscpToken.sol#96-98)
• burnFrom() (MscpToken.sol#111-118)
• name() (MscpToken.sol#127-129)
• symbol() (MscpToken.sol#135-137)
• decimals() (MscpToken.sol#148-150)
• totalSupply() (MscpToken.sol#155-157)
• balanceOf() (MscpToken.sol#162-164)
• transfer() (MscpToken.sol#173-176)
• approve() (MscpToken.sol#203-206)
• transferFrom() (MscpToken.sol#217-227)
• increaseAllowance() (MscpToken.sol#241-244)
• decreaseAllowance() (MscpToken.sol#260-268)

State variables can be declared as constant or immutable. In both cases, the variables cannot be modified after the contract has been constructed. For constant variables, the value has to be fixed at compile-time, while for immutable, it can still be assigned at construction time. The following state variables are missing the constant modifier:

CrownsToken.sol

• Line 30: uint256 public limitSupply = 1111111111000000000000000000; /// 1.1 billion

MscpToken.sol

• Line 30: uint256 public limitSupply = 1111111111000000000000000000; /// 1.1 billion