Read the findings of the 2022 Web3 Security Report

In the contract `BECoin`, the role `MINTER_ROLE` has authority over the following functions:

- `mint()`: mint tokens to any addresses

Any compromise to the `MINTER_ROLE` account may allow a hacker to take advantage of this authority.

In the contract `BECoin`, the role `PAUSER_ROLE` has authority over the following functions:

- `pause()/unpause()`: pause/unpause the contract

Any compromise to the `PAUSER_ROLE` account may allow a hacker to take advantage of this authority.

In the contract `BEBase/BEChip/BEEquipment/BEHero`, the role `owner` has authority over the following functions:

- `addApprovalWhitelist()/removeApprovalWhitelist()`: change the value of `approvalWhitelists`
- `setMintFactory()/removeMintFactory()`: add/remove the address into/from `MINTER_ROLE`
- `setBurnProxy()/removeBurnProxy()`: add/remove the address into/from `BURN_ROLE`

Any compromise to the `owner` account may allow a hacker to take advantage of this authority.

In the contract `BEBase/BEChip/BEEquipment/BEHero`, the role `BURN_ROLE` has authority over the following functions:

- `burn()`: burn the NFT token

Any compromise to the `BURN_ROLE` account may allow a hacker to take advantage of this authority.

In the contract `EvolveProxy`, the role `owner` has authority over the following functions:

- `updateExecutor()`: change the value of the `executor` to any address

Any compromise to the `owner` account may allow a hacker to take advantage of this authority.

In the contract `EvolveProxy`, the role `executor` has authority over the following functions:

- `evolveHero()`: burn the `hero` NFT and burn the `chip` NFT
- `evolveEquip()`: burn the `equip` NFT and burn the `chip` NFT

Any compromise to the `executor` account may allow a hacker to take advantage of this authority.

In the contract `Marketplace`, the role `owner` has authority over the following functions:

- `setFeeToAddress()`: change the address of the `feeToAddress`,
- `setTransactionFee()`: change the value of the variable `transactionFee`,
- `setPaymentTokens()/removePaymentTokens()`: set the allowed tokens

Any compromise to the `owner` account may allow a hacker to take advantage of this authority.

In the contract `MinterFactory`, the role `owner` has authority over the following functions:

- `allowPublicMint()`: change the value of the variable `publicMintAllowed` to be true

Any compromise to the `owner` account may allow a hacker to take advantage of this authority.

In the contract `BEGold`, the role `owner` has authority over the following functions:

- `pause/unpause()`: pause/unpause the contract `BEGold`
- `mint()`: mint any tokens to any addresses

Any compromise to the `owner` account may allow a hacker to take advantage of this authority.

Centralization Related Risks

The contract has unlocked compiler version. An unlocked compiler version in the source code of the contract permits the user to compile it at or above a particular version. This, in turn, leads to differences in the generated bytecode between compilations due to different compiler versions. This can lead to an ambiguity when debugging as compiler specific bugs may occur in the codebase that would be hard to identify over a span of multiple compiler versions rather than a specific one.

Unlocked compiler version

“public” functions that are never called by the contract should be declared “external”. When the inputs are arrays, “external” functions are more efficient than “public” functions.

Examples:

- EvolveProxy.updatePublicEvolve(bool)
- BEBoxMall.setPaymentReceivedAddress(address)
- MinterFactory.allowPublicMint()
- Marketplace.setFeeToAddress(address)
- Marketplace.setTransactionFee(uint256)
- Marketplace.setPaymentTokens(address[])
- Marketplace.removePaymentTokens(address[])
- BEBase.mint(address,uint256)
- BEBase.addApprovalWhitelist(address)
- BEBase.removeApprovalWhitelist(address)
- BEBase.setMintFactory(address)
- BEBase.removeMintFactory(address)
- BEBase.lock(uint256)
- BEBase.unlock(uint256)
- BEBase.isLocked(uint256)
- BEBase.updateBaseURI(string)
- BEBase.burn(address,uint256)
- BEBase.setBurnProxy(address)
- BEBase.removeBurnProxy(address)
- Migrations.setCompleted(uint256)
- BEGold.pause()
- BEGold.unpause()
- BEGold.mint(address,uint256)

Proper usage of “public” and “external” type

In the contract `BEBase`, the role `owner` has authority over the functions `addApprovalWhitelist()/removeApprovalWhitelist()` to change the value of the address in the variable `approvalWhitelists`.

When `approvalWhitelists[address]` is true, the address has the authority to lock/unlock the BEBase token of any tokenId.

Centralization risks of the variable `approvalWhitelists`

In the contract `ERC721Enumerable`, the view function `isApprovedForAll()` is used to check if the `operator` is allowed to manage all of the assets of `owner`.

According to the following codes, this function overrides the original function in `ERC721Enumerable`. The `approvalWhitelists[operator]` is checked at the beginning which is updated by the owner.

```solidity
    function isApprovedForAll(address owner, address operator)
        public
        view
        override
        returns (bool)
    {
        if (approvalWhitelists[operator] == true) {
            return true;
        }

        return super.isApprovedForAll(owner, operator);
    }
```

However, the `operator` can't transfer the assets of the `owner` when `approvalWhitelists[operator]` is true but `super.isApprovedForAll(owner, operator)` is false.

Logical issue of the function `isApprovedForAll()`

Variable `BECoin.initializedCap` could be declared as `constant` since the state variable is never to be changed.

Variable could be declared as `constant`

According to the following codes, the function `buyBoxWithSignature()` is called by the owner and uses the user's signature to charge tokens.

```solidity
    function buyBoxWithSignature(
        uint256 boxId,
        uint256 _type,
        address userAddress,
        uint256 price,
        address paymentErc20,
        uint256 saltNonce,
        bytes calldata signature
    ) external onlyOwner {
        require(
            !isContract(userAddress),
            "BEBoxPayment: Only user address is allowed to buy box"
        );
        require(_type > 0, "BEBoxPayment: Invalid box type");
        require(price > 0, "BEBoxPayment: Invalid payment amount");
        require(
            !usedSignatures[signature],
            "BEBoxPayment: signature used. please send another transaction with new signature"
        );
        bytes32 criteriaMessageHash = getMessageHash(
            _type,
            paymentErc20,
            price,
            saltNonce
        );

        checkSigner(userAddress, criteriaMessageHash, signature);

        IERC20 paymentToken = IERC20(paymentErc20);
        uint256 allowToPayAmount = paymentToken.allowance(
            userAddress,
            address(this)
        );
        require(
            allowToPayAmount >= price,
            "BEBoxPayment: Invalid token allowance"
        );
        // Transfer payment
        paymentToken.safeTransferFrom(
            userAddress,
            paymentReceivedAddress,
            price
        );
        // Emit payment event
        emit BEBoxPaid(boxId, userAddress, _type, price, paymentErc20);
    }
```

The `usedSignatures[signature]` is checked in the function, but will not be marked as true after the call. Besides, the function only charges the user's tokens but does not transfer the box to the user.

We would like to confirm with the client if the current implementation aligns with the original project design.

Logical issue of function `buyBoxWithSignature()`

The implementation of the `isContract` check can not cover all scenarios. The check can be bypassed if the call is from the constructor of a smart contract or when the contract is destroyed. Because, in that case, the code size will also be zero.

The "isContract" function in the OpenZeppelin "Address" library uses the same implementation, but comments mention that "it's unsafe to rely on the check and it can be bypassed". Reference: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/utils/Address.sol 

Ineffective `isContract()` check

The following functions lack a zero-check.

- `BEBoxMall.setPaymentReceivedAddress()`
- `Marketplace.setFeeToAddress()`

Missing zero address validation

Boolean constants can be used directly and do not need to be compared to true or false.

For example,

- `Marketplace.setPaymentTokens()`
- `Marketplace.matchTransaction()
- `BEBase.isApprovedForAll()`
- `BEBase.addApprovalWhitelist()`

Boolean equality

The **require** can be used to check for conditions and throw an exception if the condition is not met. It is better to provide a string message containing details about the error that will be passed back to the caller.

Missing Error Messages

According to the codes in the functions `evolveHero()/evolveEquip()`, the functions burn the `hero/equip` NFT tokens and burn the `chip` NFT tokens, but the `to` address does not gain anything.

Logical issue of the functions `evolveHero()/evolveEquip()`

100000000 `CEC` tokens are sent to the contract deployer when deploying the contract `BECoin`. This could be a centralization risk as the deployer can distribute `CEC` tokens without obtaining the consensus of the community.

100000000 `CEG` tokens are sent to the contract deployer when deploying the contract `BEGold`. This could be a centralization risk as the deployer can distribute `CEG` tokens without obtaining the consensus of the community.

Initial token distribution

According to the following codes, the function `allowPublicMint()` is used to change the value of `publicMintAllowed` to be true. In addition, the `publicMintAllowed` can never be changed back to false.

```solidity
    function allowPublicMint() public onlyOwner {
        publicMintAllowed = true;
    }
```

As a result, the users can call the functions `mintHeroTo()/mintEquipTo()/mintChipTo()` to mint NFT tokens by themselves with no restrictions at all.

Logical issue of the function `allowPublicMint()`

The following variables do not have reasonable boundaries, so they can be given arbitrarily values after deploying.

- `Marketplace.transactionFee`

Lack of reasonable boundary

CEBG Game 

BEBoxMall.sol

MarketPlace.sol

contracts

CEBG Game

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

15

0

4

2

9