CertiK reviewed NFTBOOKS on 27 Feb 2023: 22 findings, 0 critical; 16 marked fixed, 3 acknowledged, 3 open.
Report hosted on skynet.certik.com.
Trustblock does not review code.
Statuses are the security firm’s own, from its report.
| Severity | Open | Acknowledged | Fixed |
|---|---|---|---|
| Critical | 0 | 0 | 0 |
| High | 0 | 3 | 1 |
| Medium | 1 | 0 | 1 |
| Low | 2 | 0 | 14 |
When the NFT is rented after being validated, the user and expiration date can be modified using the function `buyFromRental()` of the contract `NFTMarketRental`. However, the owner or approver can directly call the function `setUser()` in the contract `NFTBookBase` to arbitrarily update the user and expiration date of the NFT without any validation. As a result, the owner of a book may frequently rent out that same book to different users throughout the same time frame.
The variable `_rOwned[account]` is not updated in the function `includeInReward()`, which will make the included accounts siphon tokens from the balances of all token holders. ```solidity=354 function includeInReward(address account) external onlyOwner() { require(_isExcluded[account], "Account is already excluded"); for (uint256 i = 0; i < _excluded.length; i++) { if (_excluded[i] == account) { _excluded[i] = _excluded[_excluded.length - 1]; _tOwned[account] = 0; _isExcluded[account] = false; _excluded.pop(); break; } } } ```
The `swapAndLiquify` function converts half of the NFTBS tokens to ETH. The other half of NFTBS tokens and part of the converted ETH are deposited into the LP pool on Uniswap as liquidity. For every `swapAndLiquify` function call, a small amount of ETH is left over in the contract. This is because the price of NFTBS drops after swapping the first half of NFTBS tokens into ETHs, and the other half of NFTBS tokens require less than the converted ETH to be paired with it when adding liquidity. The contract doesn't appear to provide a way to withdraw those ETH, and they will be locked in the contract forever.
The contract `NFTBSRolesContract` implements the interface `IRoles`, but does not inherit from it.
Addresses should be checked before assignment or external call to make sure they are not zero addresses. ```solidity=39 token = _token; ``` - `_token` is not zero-checked before being used. --- ```solidity=384 marketingWallet = payable(_marketingWallet); ``` - `_marketingWallet` is not zero-checked before being used. --- ```solidity=529 migration = _migration; ``` - `_migration` is not zero-checked before being used.
In the contract `NFTBooks`, there is a function `skipToSwapAndLiquify()` to set the variable `_isSkippedToSwapAndLiquify[account]` to true. However, there is no function to reset the variable `_isSkippedToSwapAndLiquify[account]` to false. We would like to confirm with the client if the current implementation aligns with the original project design. ```solidity=375 function skipToSwapAndLiquify(address account) public onlyOwner { require( !_isSkippedToSwapAndLiquify[account], "Account is already skipped" ); _isSkippedToSwapAndLiquify[account] = true; } ```
The function that is declared `payable` can receive ether. However, the aforementioned functions are not designed for receiving ether, so it is redundant to have the `payable` modifier.
The linked statements do not affect the functionality of the codebase and appear to be either remnants of test code or older functionality.
The `fileCID` of the book NFT can be set arbitrarily by the book purchaser or renter. We would like the team to elaborate more about the use of `fileCID`, as per the comments missing for this field in the entire codebase.
The `shares` variable defines the addresses and proportions to which the creator fee will be allocated, which will be set by the creator or publisher when initializing the book contract. We would like the team to elaborate more about `shares`, and who will get the creator fees to be distributed.
If a published version is used to create a pre-order sale that is later canceled, the creator can no longer use the published version to create a pre-order sale again because the status `existedPublicationVersions[nftContract][publicationVersion]` did not change to `false` when the order was canceled. ```solidity=274 require( !existedPublicationVersions[nftContract][publicationVersion], "NFTMarketPreorder: publicationVersion is used" ); ```
The return value of the transfer()/transferFrom() call is not checked. ```solidity=68 token.transferFrom(from, to, amount); ``` --- ```solidity=63 IERC20Upgradeable(contractRegistry.getNFTBSToken()).transfer(to, amount); ``` --- ```solidity=86 IERC20Upgradeable(token).transfer(msg.sender, transferAmount); ``` --- ```solidity=53 rewardsToken.transfer(msg.sender, amount); ```
In the contract `NFTBSBscTreasury`, the state variable `merkleRoot` stands for an off-chain Merkle tree, which is used to verify whether a given user has the privilege to claim the treasury tokens. However, the actual Merkle tree used by the contract is uncertain and can be changed. Normally, the Merkle tree is used to prevent arbitrary changes to the nodes, but the contracts allow direct updates to the Merkle tree root, which can change the whole tree. Also, there is no on-chain view method for the users to check whether they are included in the Merkle tree. This may cause confusion among users. What's more, in the function `claim()`, `getClaimableBalance()`, `getClaimableTime` the parameter `merkleProof` is passed in to verify whether the `msg.sender` is in the Merkle tree. So the actual value is uncertain and malicious users can perform a second preimage attack. We would like to confirm with the client if the current implementation aligns with the original project design.
One or more logic contracts do not protect their initializers. An attacker can call the initializer and assume ownership of the logic contract, whereby she can perform privileged operations that trick unsuspecting users into believing that she is the owner of the upgradeable contract. ```solidity=19 contract NFTBSBookFactory is IBookFactory, Initializable { ``` - `NFTBSBookFactory` is an upgradeable contract that does not protect its initializer. ```solidity=134 function initialize(address _rolesContract, address _contractRegistry) ``` - `initialize` is an unprotected initializer function. --- ```solidity=16 contract NFTBSConfig is Initializable, IConfig { ``` - `NFTBSConfig` is an upgradeable contract that does not protect its initializer. ```solidity=83 function initialize(address _rolesContract, address _contractRegistry) public initializer { ``` - `initialize` is an unprotected initializer function. --- ```solidity=13 contract NFTBSContractRegistry is IContractRegistry, Initializable { ``` - `NFTBSContractRegistry` is an upgradeable contract that does not protect its initializer. ```solidity=91 function initialize(address _rolesContract) public initializer { ``` - `initialize` is an unprotected initializer function. --- ```solidity=18 contract NFTBSMarket is ``` - `NFTBSMarket` is an upgradeable contract that does not protect its initializer. ```solidity=42 function initialize(address _rolesContract, address _contractRegistry) ``` - `initialize` is an unprotected initializer function. --- ```solidity=14 contract NFTBSRolesContract is ``` - `NFTBSRolesContract` is an upgradeable contract that does not protect its initializer. ```solidity=25 function initialize(address admin) external initializer { ``` - `initialize` is an unprotected initializer function. --- ```solidity=16 contract NFTBSTreasury is Initializable, OwnableUpgradeable { ``` - `NFTBSTreasury` is an upgradeable contract that does not protect its initializer. ```solidity=28 function initialize(address _contractRegistry) ``` - `initialize` is an unprotected initializer function. --- ```solidity=15 contract NFTBSBscTreasury is Initializable, OwnableUpgradeable { ``` - `NFTBSBscTreasury` is an upgradeable contract that does not protect its initializer. ```solidity=36 function initialize(address _token) public initializer { ``` - `initialize` is an unprotected initializer function. --- ```solidity=14 contract NFTBSRewards is Initializable, OwnableUpgradeable { ``` - `NFTBSRewards` is an upgradeable contract that does not protect its initializer. ```solidity=29 function initialize() public initializer { ``` - `initialize` is an unprotected initializer function. --- ```solidity=17 contract NFTBooks is ContextUpgradeable, IERC20Upgradeable, OwnableUpgradeable { ``` - `NFTBooks` is an upgradeable contract that does not protect its initializer. ```solidity=138 function initialize( ``` - `initialize` is an unprotected initializer function.
```solidity=129 _serviceFeeForPreorder = uint16(serviceFeeForPreorder); ``` - The type conversion `uint16(serviceFeeForPreorder)` from larger type uint256 to smaller type uint16 may truncate data. --- ```solidity=140 _serviceFeeForSell = uint16(serviceFeeForSell); ``` - The type conversion `uint16(serviceFeeForSell)` from larger type uint256 to smaller type uint16 may truncate data. --- ```solidity=151 _serviceFeeForRental = uint16(serviceFeeForRental); ``` - The type conversion `uint16(serviceFeeForRental)` from larger type uint256 to smaller type uint16 may truncate data. --- ```solidity=167 _maxPublicationOpeningRange = uint64(maxPublicationOpeningRange); ``` - The type conversion `uint64(maxPublicationOpeningRange)` from larger type uint256 to smaller type uint64 may truncate data. --- ```solidity=178 _minRoyaltyForSell = uint16(minRoyaltyForSell); ``` - The type conversion `uint16(minRoyaltyForSell)` from larger type uint256 to smaller type uint16 may truncate data. --- ```solidity=189 _maxRoyaltyForSell = uint16(maxRoyaltyForSell); ``` - The type conversion `uint16(maxRoyaltyForSell)` from larger type uint256 to smaller type uint16 may truncate data. --- ```solidity=200 _minRoyaltyForRental = uint16(minRoyaltyForRental); ``` - The type conversion `uint16(minRoyaltyForRental)` from larger type uint256 to smaller type uint16 may truncate data. --- ```solidity=211 _maxRoyaltyForRental = uint16(maxRoyaltyForRental); ``` - The type conversion `uint16(maxRoyaltyForRental)` from larger type uint256 to smaller type uint16 may truncate data. --- ```solidity=227 _minNumberBooksForPublication = uint64(minNumberBooksForPublication); ``` - The type conversion `uint64(minNumberBooksForPublication)` from larger type uint256 to smaller type uint64 may truncate data. --- ```solidity=243 _maxNumberBooksForPublication = uint64(maxNumberBooksForPublication); ``` - The type conversion `uint64(maxNumberBooksForPublication)` from larger type uint256 to smaller type uint64 may truncate data. --- ```solidity=259 _minTimeToExtendBeforeRentalExpired = uint64(minTimeToExtendBeforeRentalExpired); ``` - The type conversion `uint64(minTimeToExtendBeforeRentalExpired)` from larger type uint256 to smaller type uint64 may truncate data. --- ```solidity=275 _minAllowedTimeToRepublish = uint64(minAllowedTimeToRepublish); ``` - The type conversion `uint64(minAllowedTimeToRepublish)` from larger type uint256 to smaller type uint64 may truncate data. --- ```solidity=52 info.expires = uint64(expires); ``` - The type conversion `uint64(expires)` from larger type uint256 to smaller type uint64 may truncate data. --- ```solidity=39 _totalPublications = uint64(totalPublications); ``` - The type conversion `uint64(totalPublications)` from larger type uint256 to smaller type uint64 may truncate data. --- ```solidity=41 _royaltyFeeForSell = uint16(royaltyFeeForSell); ``` - The type conversion `uint16(royaltyFeeForSell)` from larger type uint256 to smaller type uint16 may truncate data. --- ```solidity=42 _royaltyFeeForRental = uint16(royaltyFeeForRental); ``` - The type conversion `uint16(royaltyFeeForRental)` from larger type uint256 to smaller type uint16 may truncate data. --- ```solidity=53 _totalPublications = uint64(totalPublications); ``` - The type conversion `uint64(totalPublications)` from larger type uint256 to smaller type uint64 may truncate data. --- ```solidity=224 preorderSale.openDate = uint64(openDate); ``` - The type conversion `uint64(openDate)` from larger type uint256 to smaller type uint64 may truncate data. --- ```solidity=225 preorderSale.closeDate = uint64(closeDate); ``` - The type conversion `uint64(closeDate)` from larger type uint256 to smaller type uint64 may truncate data. --- ```solidity=226 preorderSale.quantityLimit = uint64(quantityLimit); ``` - The type conversion `uint64(quantityLimit)` from larger type uint256 to smaller type uint64 may truncate data.
The return value of an external call is not stored in a local or state variable. ```solidity=343 IBookContract(nftContract).updateTotalPublications(totalPublications); ``` --- ```solidity=178 IBookContract(nftContract).updateTokenFileCID(tokenId, fileCID); ``` --- ```solidity=146 IBookContract(nftContract).updateTokenFileCID(tokenId, fileCID); ``` --- ```solidity=283 IBookContract(nftContract).updateTokenFileCID(tokenId, fileCID); ``` --- ```solidity=855 uniswapV2Router.addLiquidityETH{value: maticAmount}( address(this), tokenAmount, 0, // slippage is unavoidable 0, // slippage is unavoidable owner(), block.timestamp ); ```
The report names a repository or files. Open the report to see its scope.
Repository and files the report names (35)
1,044 projects reviewed · 1,375 reports · latest May 2025
See CertiK’s auditsSomething wrong on this page? A missing report, a wrong date, a finding that was fixed since. Tell us and we check it against its source.
Tell us