Ninja Spin - Seascape

The BigBangNFT.sol contract constructor contains an increment of the counter variable, making the IDs begin on 1.

ScapeStore.sol

    constructor() public ERC721("BigBang NFT", "BB") {
        nftId.increment();
    }

\color{black} \color{white}

It is known that some collections start at the NFT ID 0.

SOLVED: The Ninja Scape Game can now start minting NFTs at ID 0. This issue was fixed in commit ID 63f46b35467cd3865416b1dae2aa99ea4363cf63

In the _safeTransfer() function from BigBangGame.sol smart contract, two different types of transfers could be performed. if _token address sent as parameter is different from 0, the function will perform a token.transfer(). If _token is address(0), then the contract will send the native coin to the sender.

It is not possible to add address(0) to the list of allowed tokens, as the addresses are validating this parameter to be different from 0. This makes it impossible to operate with native tokens.

Code Location

BigBangGame.sol

constructor(address _token, address _nft, address _factory, address _verifier) public {
    require(_token != address(0), "BBGame: Token can't be zero address");
    require(_nft != address(0), "BBGame: Nft can't be zero address");
    require(_verifier != address(0), "BBGame: Verifier can't be zero address");

    BBNft     = _nft;
    BBFactory = _factory;
    verifier    = _verifier;

    changeAllowed[_token] = true;
    token[typeId]     = _token;
  }

BigBangGame.sol

  function addToken(address _token) public onlyOwner {
    require(_token != address(0), "BBGame: Token can't be zero address");
    require(!changeAllowed[_token], "BBGame: This token is exist");

    changeAllowed[_token] = true;
    token[++typeId] = _token;

    emit AddToken(_token, typeId, block.timestamp);
  }

BigBangGame.sol

  function _safeTransfer(address _token, address _to, uint256 _amount) internal {
    if (_token != address(0)) {
      IERC20 _rewardToken = IERC20(_token);

      uint256 _balance = _rewardToken.balanceOf(address(this));
      require(_amount <= _balance, "BBGame: Do not have enough token to reward");

      uint256 _beforBalance = _rewardToken.balanceOf(_to);
      _rewardToken.transfer(_to, _amount);

      require(_rewardToken.balanceOf(_to) == _beforBalance + _amount, "BBGame: Invalid transfer");
    } else {

      uint256 _balance = address(this).balance;
      require(_amount <= _balance, "BBGame: Do not have enough token to reward");

      payable(_to).transfer(_amount);
    }
  }

  // Accept native tokens.
  receive() external payable {
    //React to receiving ether
  }

SOLVED: The SeaScape Team now allows the BigBangGame.sol contract to add the address(0) to specify that the native token could be used. This issue was fixed in commit ID 6fbb5845900735349c19c5e751991b9854d9a2a5

There is no validation of the addresses in the mint() and access control functions. Addresses should be validated and checked that are different from zero when necessary. This issue is present in all the smart contracts, in the constructors and functions that use addresses as parameters. These examples show how the factory could be set up with a wrong address and how mint could burn NFTs if _owner is address(0)

BigBangNFTFactory.sol

    constructor(address _nft) public {
        nft = BigBangNFT(_nft);
        _setupRole(DEFAULT_ADMIN_ROLE, msg.sender);
    }

BigBangNFTFactory.sol

   function mint(address _owner, uint256 _quality, uint256 _image) public onlyGenerator returns(uint256) {
      require (_quality > 0 && _quality < 6, "NFT Factory: invalid quality");
      return nft.mint(_owner, _quality, _image);
   }

SOLVED: The SeaScape Team now checks the address inputs to ensure they are non-zero. This issue has been fixed in commit ID [7d4d7a60ef10d499be2c672a6b623211ef288246](https://github.com/Seastarinteractive/moonscape-smartcontracts/tree/7d4d7a60ef10d499be2c672a6b623211ef288246

There are functions whose parameters are never used in some smart contracts. These parameters have no effect on the code.

Code Location

• operator, from, tokenId, data (BigBangGame.sol#190)
• BBFactory (BigBangGame.sol#231)

ACKNOWLEDGED: The SeaScape team acknowledged this finding.

The BigBangGame smart contracts use the pragma version 0.6.7 which was released on May 4, 2020. latest pragma version 0.8.16 released on August 8, 2022, solves different issues. It is also noticeable that Solidity versions after 0.8.0 also implement default overflow protection on arithmetic operations.

Reference: Solidity Releases

Code Location

note: All Ninja Spin smart contracts implement same pragma version.

BigBangGame.sol

pragma solidity 0.6.7;

ACKNOWLEDGED: The SeaScape team acknowledged this finding.

Testing revealed that calculations within the smart contract did not make use of a safe math library. While Solidity pragma > 0.8.0 reverts to overflows by default, the code was making use of an older compiler version; thus it is vulnerable to integer overflows and underflows.

Halborn could not find a path to exploit the integer overflows; however, there might be some extreme test cases where this can be exploited by an attacker.

Code Location

```{language=solidity caption="BigBangGame.sol" firstnumber=128 hlines=148} function goldChangeToken(uint256 _gold, uint256 _typeId, uint8 _v, bytes32 _r, bytes32 _s) external { require(_gold > 0, "BBGame: The exchange amount must greater than zero"); require(checkToken(_typeId), "BBGame: Do not have this token type");

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

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

  require(recover == verifier, "BBGame: Verification failed about goldChangeToken");
}

nonce[msg.sender]++;

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

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

} ```

ACKNOWLEDGED: The SeaScape team acknowledged this finding.