Read the findings of the 2022 Web3 Security Report

Multiple roles are adopted to enforce access control. The advantage of the roles in the codebase is that the client reserves the ability to adjust the protocol according to the runtime required to best serve the community. It is also worthy of note the potential drawbacks of these functions, which should be clearly stated through the client's action/plan. Additionally, if the private keys of the privileged accounts are compromised, it could lead to a devastating consequence to the project.

In the contract **`Fee`**, the role `OPS_ROLE` has authority over the following functions:
- `setDefaultFee()`: update the default value related to fees.
- `setFeeFor()`: set the fee values of an arbitrary contract.
- `removeFeeFor()`: remove the fee values of an arbitrary contract.

Additionally, `DEFAULT_ADMIN_ROLE` will be assigned to the `Fee` contract deployer, the role can control the role assignment for the `Fee` contract.

In the contract **`ManageToken`**, the role `_owner` has the authority over the following functions:
- `addToken()`: add an arbitrary token as the supported token.
- `deleteToken()`: delete an arbitrary token as the supported token.
- `transferOwnership()`: transfer contract ownership to new owner.
- `renounceOwnership()`: transfer contract ownership to address(0).

In the contract **`TokoMarketplace`**, the role `DEFAULT_ADMIN_ROLE` has the authority over the following functions:
- `pause()`/`unpause()`: pause/unpause major functions of the contract.
- `setManagement()`: update platform fee value, management token address and fee address.

Additionally, the role `OPS_ROLE` has the authority over the following functions:
- `addBannedAddress()`/`deleteBannedAddress()`: include/exclude an address to/from the `banned` list.

In the contract **`TKONFTMarketplace`**, the role `DEFAULT_ADMIN_ROLE` has the authority over the following functions:
- `setIFee`: update the address that calculate fees.
- `pause()`/`unpause()`: pause/unpause major functions of the contract.
- `setFeeAddress()`: set an arbitrary address to receive fee.
- `setExpiredTimes()`: set the expire time.

Additionally, the role `OPS_ROLE` has the authority over the following functions:
- `addContractNFT()`/`removeContractNFT()`: add/remove the nft contracts for the marketplace.
- `suspendCollector()`/`suspendCollector()`: suspend/unsuspend an arbitrary address to interact with the contract.
- `suspendNFTBatch()`/`unsuspendNFTBatch()`: suspend/unsuspend NFT contracts.

In the contract **`PriceFeedTKOBIDR`**, the role `_owner` has the authority over the following functions:
- `updateDecimals()`: update the decimals of the oracle.
- `updateDescription()`: update the description of the oracle.
- `updateVersion()`: update the version of the oracle.
- `updatePrice()`: update the price of the oracle.
- `transferOwnership()`: transfer contract ownership to new owner.
- `renounceOwnership()`: transfer contract ownership to address(0).


In the contract **`TKONFTBlindBoxSet`**, the role `_owner` has the authority over the following functions:
- `safeMintBatch()`: mint blind box nft tokens in a batch.
- `burnBatch()`: burn blind box nft tokens.
- `transferOwnership()`: transfer contract ownership to new owner.
- `renounceOwnership()`: transfer contract ownership to address(0).


In the contract **`TKONFTMerchant`**, the role `_owner` has the authority over the following functions:
- `safeMint()`: mint an nft token to an address with given metadata.
- `safeMintBatch()`: mint nft tokens in a batch.
- `transferOwnership()`: transfer contract ownership to new owner.
- `renounceOwnership()`: transfer contract ownership to address(0).


In the contract **`TKONFT`**, the role `DEFAULT_ADMIN_ROLE` has the authority over the following functions:
- `addOpsRole()`: grant an address as `OPS_ROLE` role.

The role `OPS_ROLE` has the authority over the following functions:
- `addMintRole()`: grant a role as `MINTER_ROLE`.
- `newVerify()`: verify a given `tokenId`.
- `newVerifyBatch()`: verify tokens in a batch.
- `removeVerify()`: remove a given `tokenId` from the `verify` list.
- `removeVerifyBatch()`: remove `tokenId`s from the `verify` list in a batch.

The role `MINTER_ROLE` has the authority over the following functions:
- `safeMint()`: mint an nft with given data.
- `safeMintBatch()`: mint nfts in a batch.

Any compromise to the aforementioned privileged accounts may allow a hacker to take advantage of this authority and manipulate the protocol.

Centralization Related Risks

The contract has an 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 differing compiler version numbers. This can lead to 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

A reentrancy attack can occur when the contract creates a function that makes an external call to another 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. 

In the `TKONFTMarketplace` contract, functions `sellNFTBatch()`/`cancelSellNFTBatch()`/`buyNFT()` will invoke `safeTransferFrom()` function of an IERC721 implemented contract. According to the [openzeppelin implementation](https://docs.openzeppelin.com/contracts/4.x/api/token/erc721), `safeTransferFrom()` would trigger the `IERC721Receiver.onERC721Received()` function of the receiver contract. However, this external function call creates a security loophole. Specifically, the attacker can perform a reentrant call inside the `onERC721Received()` callback.

Therefore, it is possible for an attacker to create a malicious `onERC721Received()` callback and reenter the `TKONFTMarketplace` contract. For example, 
```solidity=199
IERC721(NFTSeller._contractNFT).safeTransferFrom(address(this), sender, NFTSeller._tokenId);
if (_firstSellingTokens[NFTSeller._contractNFT][NFTSeller._tokenId] != address(0) && NFTSeller._firstSellingMerchant) {
    delete _firstSellingTokens[NFTSeller._contractNFT][NFTSeller._tokenId];
}
delete _numAskSellNFT[numAsks_[i]];
delete _NFTSellers[numAsks_[i]];
emit CancelSellNFT(numAsks_[i], sender, NFTSeller._contractNFT, NFTSeller._tokenId);
```
In the `cancelSellNFTBatch()` function, the `safeTransferFrom()` is called before the corresponding record (i.e., `_numAskSellNFT` and `_NFTSellers`) is deleted. Therefore, the attacker could re-enter the `buyNFT()` functions before the `cancelSellNFTBatch()` function is completed. It could lead to unexpected behaviour of the project and introducing the reentrancy attack vectors.

Additionally, in the contract `TKONFTBlindBoxSet`, the functions `safeMintBatch()`/`burnBatch()`/`openBoxBatch()` also invoke external calls from unknow implementation.

Potential Reentrancy Issues

Mathematical operations in the aforementioned line perform divisions before multiplications. Performing multiplication before division can sometimes avoid loss of precision. For example,
```solidity=125
        uint256 _platformFee = order_.price / 1e4 * platformFee;
        uint256 _royalty = royaltyInfoFee.royalty > 0 ? order_.price / 1e4 * royaltyInfoFee.royalty : 0;
```
The above calculations divide `1e4` before multiplying `platformFee`, which is potentially resulting in a loss of precision.

Division Before Multiplication

The `versionCode` only serves as a getter function (return its value) and performs no actual functionality. 

Unused Logic 

The price acquired from the `PriceFeedTKOBIDR` relies on owner's input. The owner is responsible for the price update via the function `updatePrice()`.
```solidity=45
    function updatePrice(int256 _answer) external onlyOwner {
        uint256 times = block.timestamp;
        tkobidr[latestRoundId] = pricefeed(_answer, times);
        emit Price(latestRoundId, _answer, times);
        latestRoundId += 1;
    }
```
The concern is considering human error, the price could be inaccurate. Additionally, if the owner's private key is compromised, the price could be inaccurate, even being manipulated.

Oracle Price Relies on Manual Update

When receiving a blind box nft, users need to wait for a locking period before "opening" the blind box and get the nft "in the box" via `openBoxBatch()`.
```solidity=92
    function openBoxBatch(uint256[] memory tokenIds) external {
        require(tokenIds.length > 0, "ERROBB1");
        address sender = _msgSender();
        for (uint256 i = 0; i < tokenIds.length; i++) {
            require(block.timestamp > lockBox[tokenIds[i]], "ERROBB2");
```
However, this locking period can be bypassed via a reentrancy attack.

A reentrancy attack can occur when the contract creates a function that makes an external call to another 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. 

In the current project, `lockBox[tokenId]` is updated after the `safeTransferFrom()` external calls. Therefore, there is a potential attack vector to reenter the contract and bypass the checking of `lockBox`. As mentioned in the previous reentrancy issues, the Openzeppelin's `safeTransferFrom()` will invoke the `onERC721Received()` of the receiver (if the receiver is a contract). In this case, the attacker can perform an `openBoxBatch()` invocation in the `onERC721Received()` contract to reenter the contract, thus bypassing the lock validation in `openBoxBatch()`.


Exploit scenario:
1. The attacker receives some blind box nfts via the `safeTransferFrom()` invocation. In normal cases, the attacker needs to wait a locking period before calling `openBoxBatch()` to get the NFT.
2. However, the attacker can deploy a contract as the receiver and implement an `onERC721Received()` contract that invokes the `openBoxBatch()` function.
3. Since the `lockBox[tokenId]` is updated after the `openBoxBatch()` invocation, the lock validation will be bypassed.
4. Therefore, the attacker can open the blind box and get the NFT without waiting.


Potential Bypass of the Locking Period

It is now well understood that the difficulty or timestamp is not a source of randomness. They can be manipulated if the attacker is also a miner. For EVM compatible blockchains such as Binance smart chain and Polygon, the `block.difficulty` and `block.gaslimit` can be the same value across multiple blocks, making the situation even worse. 

Risk For Weak Randomness

Tokomall

NftFactory.sol

price-feed-tko-bidr.sol

Fee.sol

ManageToken.sol

token-nft-blind-box-set-oz.sol

TokoMarketplace.sol

Migrations.sol

token-nft-commerce-erc721-oz.sol

IBEP20.sol

token-nft-commerce-erc721-merchant-oz.sol

BlindBoxSetFactory.sol

nft-marketplace-oz.sol

Centralization Risk

Voting Power Not Moved Along With Transfer

Toko TKO

TKOToken.sol

A BEP-20 token contract with governance delegration, undelegation capability

Incorrect burn flow

Centralized Risk

Toko Token (TKO) is the first local Indonesian Crypto Asset Project to offer a unique hybrid token model.

Tokocrypto

Highlights & Alerts

This is just a placeholder

highlight placeholder lorem Ipsum...

highlight lorem

This is a alert demo

Alert demo

Code Audit History

8

0

8

0

0