VLXNFTExchange

Smart Contract Audit Report

Audit Summary

VLXNFTExchange Audit Report KitcheNFT is building a new unique NFT collection along with a marketplace that allows users to buy, sell, and transfer NFTs.

For this audit, we reviewed the project team's VLXNFT and ExchangeNFT contracts at commit 362641d1ef2997de75a3a69d8c81437a268f07df on the team's private GitHub repository.

Audit Findings

Please ensure trust in the team prior to investing as they have substantial control in the ecosystem.
Date: March 29th, 2022.
Updated: April 8th, 2022 to reflect changes made by the team.
Updated: April 13th, 2022 to reflect changes made by the team.
Updated May 2nd, 2022 to reflect changes made by the team.
Finding #1 - ExchangeNFT - High (Resolved)
Description: Signed messages can successfully pass the verify() function without any unique values corresponding to order details within the buyToken(), sellToken(), and cancelOrder() functions.
Risk/Impact: Any signed message from the seller address can be used to satisfy the verify() function call and initiate a process not intended by the Signer address.
Recommendation: The order details, a unique single-use nonce value, and the msg.sender address should be hashed together to re-construct the signed message on-chain, thereby confirming the signed message can only be used for a specific order.
Resolution: The team has implemented the order details and a unique order ID that cannot be used more than once. The team does not wish to implement the msg.sender address so any address with the signed message will be able to complete the transaction.
Finding #2 - VLXNFT - High (Resolved)
Description: The mint() function uses a signed message to determine the Signer address that is minted the corresponding VLXNFT NFT and set as the owner; the signed message serves no purpose other than determining the Signer address.
Risk/Impact: Any signed message can be provided in order to determine a Signer address within the mint() function.
Recommendation: The address to be minted to should be stored within the Voucher struct instead of being determined by the unnecessary signed message logic.
Resolution: The mint() function no longer requires a signed message to determine the token creator.
Finding #3 - VLXNFT - High (Resolved)
Description: The setTokenURI() function allows any user to set the URI value of any NFT at any time.
Risk/Impact: Users can change any NFT's URI value at any time, rendering each NFT worthless.
Recommendation: The team should implement permission controls to limit which users can change NFT URI values.
Resolution: The setTokenURI() function has been removed.
Finding #4 - ExchangeNFT - Medium (Resolved)
Description: An order's royalty percentage is not required to be equal to the royalty percentage set when the NFT was minted.
Risk/Impact: Users can avoid paying a creator royalty when exchanging NFTs by setting the order's royalty to 0.
Recommendation: Logic should be implemented to require the order's royalty percentage to be equal to the token's set royalty percentage.
Resolution: The team has implemented logic to confirm the tokens correct royalty amount and creator address are used within the transaction.
Finding #5 - ExchangeNFT - High (Resolved)
Description: The cancelOrder() function does not stop users from using the contract to fulfill orders given that the appropriate signature is used.
Risk/Impact: Users can use the contract to complete an order after the order's maker address has used the cancelOrder() function in an attempt to cancel it.
Recommendation: Upon canceling an order the nonce associated with the order should be marked as used invalidating any signed message.
Resolution: The team has removed the cancelOrder() function from the contract.
Finding #6 - ExchangeNFT - Low (Resolved)
Description: The sellToken() function calls the verify() function to confirm the seller address is the Signer address of the message while also redundantly requiring that the caller of the function is the order's seller address.
Recommendation: The signature verification process is repetitive and unnecessary as the function already requires that the caller is the order's seller address and can be removed to save on gas costs and reduce contract size.
Resolution: The sellToken() function no longer requires a signed message from the caller as recommended above.
Finding #7 - ExchangeNFT - Low (Resolved)
Description: The cancelOrder() function calls the verify() function to confirm the seller address is the Signer address of the message while also redundantly requiring that the caller of the function is the order's maker address.
Recommendation: The signature verification process is repetitive and unnecessary as the function already requires the caller is the order's seller address and can be removed to save on gas costs and reduce contract size.
Resolution: The team has removed the cancelOrder() function from the contract.
Finding #8 - ExchangeNFT - Low (Resolved)
Description: The calculations of the _marketValue and _creatorValue variables perform multiplication on the result of a division within the buyToken() function.
Risk/Impact: This can lead to slightly less accurate results for the _marketValue and _creatorValue variables.
Recommendation: We recommend first performing multiplication operations and then division operations within variable calculations.
Resolution: The order of operations has been corrected for more accurate results.
Finding #9 - ExchangeNFT - Low (Resolved)
Description: The calculations of the _marketValue and _creatorValue variables perform multiplication on the result of a division within the sellToken() function.
Risk/Impact: This can lead to slightly less accurate results for the _marketValue and _creatorValue variables.
Recommendation: We recommend first performing multiplication operations and then division operations within variable calculations.
Resolution: The order of operations has been corrected for more accurate results.
Finding #10 - VLXNFTExchange - Informational (Resolved)
Description: Several functions are declared public, but are never called internally.
ExchangeNFT.compareStringsbyBytes, VLXNFT.mint, VLXNFT.getCreator, VLXNFT.burn, VLXNFT.approveBulk, VLXNFT.getApprovedBulk
Recommendation: We recommend declaring these functions external for additional gas savings on each call.
Resolution: The above functions have either been removed or declared external.

Contract Overview

As the contracts are implemented with Solidity v0.8.X, they are protected from any underflow/overflow attacks.
VLXNFT Contract:
This contract allows users to mint VLXNFT NFTs with a user-specified URI value.
On minting, the creator address, royalty amount, and URI value are set for the token; the royalty amount is taken as a percentage of the sale price if the NFT is sold and transferred to the creator address.
Users can grant approval permission to specific addresses for their NFTs.
A burn function is present allowing users to burn their NFTs or NFTs they've been granted approval permission over at any time.
The owner can transfer ownership at any time.
The contract complies with the ERC-721 standard.
ExchangeNFT Contract:
This contract enables users to buy, sell, and transfer VLXNFT NFTs using gasless signed messages; signed messages are stored on the team server and rely on off-chain logic.
Users can transfer their VLXNFT NFTs to any address at any time as long as the referenced VLXNFT contract address has been added to the contract's NFT collection address list.
Users can submit a buy order for a specified VLXNFT NFT for a predefined amount of ETH.
For the buy order submission to be successful, the following criteria must be met:
The order ID must not have been marked completed.
The signature and reconstructed signed message must match the order's seller address.
The caller must be the order's buyer and maker address and cannot be the order's seller address.
The amount of ETH sent must be at least the order's price amount; any excess ETH is returned to the user.
The order's seller address must be the owner of the VLXNFT NFT.
The order's expiration date must not be reached.
The order's NFT contract address must exist within the contract's NFT collection address list.
Users can submit a sell order for their VLXNFT NFT using a predefined amount of a team designated token.
For the sell order submission to be successful, the following criteria must be met:
The signature and reconstructed message must match the order's seller address.
The caller address must be the order's seller and maker address and cannot be the order's buyer address.
The caller must be the owner of the VLXNFT NFT.
The order's expiration date must not be reached.
The order's NFT contract address must exist within the contract's NFT collection address list.
There is a market service fee and creator royalty fee charged on all buy and sell orders; the marketing fee is set to 2.75% by default while the creator royalty fee is specific to each order's token royalty amount.
The market service fee is divided equally between the two fee addresses while the creator royalty fee is transferred to the creator address listed for the order's token ID from the VLXNFT contract.
Once an NFT exchange is complete, the order ID is marked as completed and cannot be used again.
Any user can add a VLXNFT NFT collection address to the NFT collection list so it can be used within the contract.
The owner can transfer ownership at any time.
The owner can set the fee addresses at any time.
The owner can set the marketing fee amount at any time.

Audit Results

Vulnerability Category	Notes	Result
Arbitrary Jump/Storage Write	N/A	PASS
Centralization of Control	The owner can set the market service fee to any amount at any time.	WARNING
Compiler Issues	N/A	PASS
Delegate Call to Untrusted Contract	N/A	PASS
Dependence on Predictable Variables	N/A	PASS
Ether/Token Theft	N/A	PASS
Flash Loans	N/A	PASS
Front Running	N/A	PASS
Improper Events	N/A	PASS
Improper Authorization Scheme	N/A	PASS
Integer Over/Underflow	N/A	PASS
Logical Issues	N/A	PASS
Oracle Issues	N/A	PASS
Outdated Compiler Version	N/A	PASS
Race Conditions	N/A	PASS
Reentrancy	N/A	PASS
Signature Issues	N/A	PASS
Unbounded Loops	N/A	PASS
Unused Code	N/A	PASS
Overall Contract Safety		PASS

VLXNFT Contract

Smart Contract Audit - Inheritance

Smart Contract Audit - Graph


 ($) = payable function
 # = non-constant function
 
 Int = Internal
 Ext = External
 Pub = Public
 
+ [Int] IAccessControl 
    - [Ext] hasRole
    - [Ext] getRoleAdmin
    - [Ext] grantRole #
    - [Ext] revokeRole #
    - [Ext] renounceRole #

 +  Context 
    - [Int] _msgSender
    - [Int] _msgData

 + [Lib] Strings 
    - [Int] toString
    - [Int] toHexString
    - [Int] toHexString

 + [Int] IERC165 
    - [Ext] supportsInterface

 +  ERC165 (IERC165)
    - [Pub] supportsInterface

 +  AccessControl (Context, IAccessControl, ERC165)
    - [Pub] supportsInterface
    - [Pub] hasRole
    - [Int] _checkRole
    - [Pub] getRoleAdmin
    - [Pub] grantRole #
       - modifiers: onlyRole
    - [Pub] revokeRole #
       - modifiers: onlyRole
    - [Pub] renounceRole #
    - [Int] _setupRole #
    - [Int] _setRoleAdmin #
    - [Int] _grantRole #
    - [Int] _revokeRole #

 + [Int] IERC721 (IERC165)
    - [Ext] balanceOf
    - [Ext] ownerOf
    - [Ext] safeTransferFrom #
    - [Ext] transferFrom #
    - [Ext] approve #
    - [Ext] getApproved
    - [Ext] setApprovalForAll #
    - [Ext] isApprovedForAll
    - [Ext] safeTransferFrom #

 + [Int] IERC721Receiver 
    - [Ext] onERC721Received #

 + [Int] IERC721Metadata (IERC721)
    - [Ext] name
    - [Ext] symbol
    - [Ext] tokenURI

 + [Lib] Address 
    - [Int] isContract
    - [Int] sendValue #
    - [Int] functionCall #
    - [Int] functionCall #
    - [Int] functionCallWithValue #
    - [Int] functionCallWithValue #
    - [Int] functionStaticCall
    - [Int] functionStaticCall
    - [Int] functionDelegateCall #
    - [Int] functionDelegateCall #
    - [Int] verifyCallResult

 +  ERC721 (Context, ERC165, IERC721, IERC721Metadata)
    - [Pub]  #
    - [Pub] supportsInterface
    - [Pub] balanceOf
    - [Pub] ownerOf
    - [Pub] name
    - [Pub] symbol
    - [Pub] tokenURI
    - [Int] _baseURI
    - [Pub] approve #
    - [Pub] getApproved
    - [Pub] setApprovalForAll #
    - [Pub] isApprovedForAll
    - [Pub] transferFrom #
    - [Pub] safeTransferFrom #
    - [Pub] safeTransferFrom #
    - [Int] _safeTransfer #
    - [Int] _exists
    - [Int] _isApprovedOrOwner
    - [Int] _safeMint #
    - [Int] _safeMint #
    - [Int] _mint #
    - [Int] _burn #
    - [Int] _transfer #
    - [Int] _approve #
    - [Int] _setApprovalForAll #
    - [Prv] _checkOnERC721Received #
    - [Int] _beforeTokenTransfer #
    - [Int] _afterTokenTransfer #

 +  Pausable (Context)
    - [Pub]  #
    - [Pub] paused
    - [Int] _pause #
       - modifiers: whenNotPaused
    - [Int] _unpause #
       - modifiers: whenPaused

 +  ERC721Pausable (ERC721, Pausable)
    - [Int] _beforeTokenTransfer #

 +  ERC721URIStorage (ERC721)
    - [Pub] tokenURI
    - [Int] _setTokenURI #
    - [Int] _burn #

 +  Ownable (Context)
    - [Pub]  #
    - [Pub] owner
    - [Pub] renounceOwnership #
       - modifiers: onlyOwner
    - [Pub] transferOwnership #
       - modifiers: onlyOwner
    - [Int] _transferOwnership #

 + [Lib] Counters 
    - [Int] current
    - [Int] increment #
    - [Int] decrement #
    - [Int] reset #

 +  VLXNFT (ERC721URIStorage, AccessControl, Ownable)
    - [Pub]  #
       - modifiers: ERC721
    - [Pub] getMessageHash
    - [Pub] getEthSignedMessageHash
    - [Pub] verify
    - [Pub] recoverSigner
    - [Pub] splitSignature
    - [Pub] mint #
    - [Pub] getCreator
    - [Pub] burn #
    - [Int] _baseURI
    - [Pub] supportsInterface
    - [Pub] setTokenURI #
    - [Pub] approveBulk #
    - [Pub] getApprovedBulk

ExchangeNFT Contract

Smart Contract Audit - Inheritance

Smart Contract Audit - Graph


 ($) = payable function
 # = non-constant function
 
 Int = Internal
 Ext = External
 Pub = Public
 
 + [Lib] SafeMath 
    - [Int] tryAdd
    - [Int] trySub
    - [Int] tryMul
    - [Int] tryDiv
    - [Int] tryMod
    - [Int] add
    - [Int] sub
    - [Int] mul
    - [Int] div
    - [Int] mod
    - [Int] sub
    - [Int] div
    - [Int] mod

 + [Lib] Math 
    - [Int] max
    - [Int] min
    - [Int] average
    - [Int] ceilDiv

 +  Context 
    - [Int] _msgSender
    - [Int] _msgData

 +  Ownable (Context)
    - [Pub]  #
    - [Pub] owner
    - [Pub] renounceOwnership #
       - modifiers: onlyOwner
    - [Pub] transferOwnership #
       - modifiers: onlyOwner
    - [Int] _transferOwnership #

 + [Int] IERC20 
    - [Ext] totalSupply
    - [Ext] balanceOf
    - [Ext] transfer #
    - [Ext] allowance
    - [Ext] approve #
    - [Ext] transferFrom #

 + [Lib] Address 
    - [Int] isContract
    - [Int] sendValue #
    - [Int] functionCall #
    - [Int] functionCall #
    - [Int] functionCallWithValue #
    - [Int] functionCallWithValue #
    - [Int] functionStaticCall
    - [Int] functionStaticCall
    - [Int] functionDelegateCall #
    - [Int] functionDelegateCall #
    - [Int] verifyCallResult

 + [Lib] SafeERC20 
    - [Int] safeTransfer #
    - [Int] safeTransferFrom #
    - [Int] safeApprove #
    - [Int] safeIncreaseAllowance #
    - [Int] safeDecreaseAllowance #
    - [Prv] _callOptionalReturn #

 + [Int] IERC165 
    - [Ext] supportsInterface

 + [Int] IERC721 (IERC165)
    - [Ext] balanceOf
    - [Ext] ownerOf
    - [Ext] safeTransferFrom #
    - [Ext] transferFrom #
    - [Ext] approve #
    - [Ext] getApproved
    - [Ext] setApprovalForAll #
    - [Ext] isApprovedForAll
    - [Ext] safeTransferFrom #

 + [Int] IERC721Receiver 
    - [Ext] onERC721Received #

 +  ERC721Holder (IERC721Receiver)
    - [Pub] onERC721Received #

 +  Pausable (Context)
    - [Pub]  #
    - [Pub] paused
    - [Int] _pause #
       - modifiers: whenNotPaused
    - [Int] _unpause #
       - modifiers: whenPaused

 + [Int] IAccessControl 
    - [Ext] hasRole
    - [Ext] getRoleAdmin
    - [Ext] grantRole #
    - [Ext] revokeRole #
    - [Ext] renounceRole #

 + [Lib] Strings 
    - [Int] toString
    - [Int] toHexString
    - [Int] toHexString

 +  ERC165 (IERC165)
    - [Pub] supportsInterface

 +  AccessControl (Context, IAccessControl, ERC165)
    - [Pub] supportsInterface
    - [Pub] hasRole
    - [Int] _checkRole
    - [Pub] getRoleAdmin
    - [Pub] grantRole #
       - modifiers: onlyRole
    - [Pub] revokeRole #
       - modifiers: onlyRole
    - [Pub] renounceRole #
    - [Int] _setupRole #
    - [Int] _setRoleAdmin #
    - [Int] _grantRole #
    - [Int] _revokeRole #

 + [Int] IERC721Metadata (IERC721)
    - [Ext] name
    - [Ext] symbol
    - [Ext] tokenURI

 +  ERC721 (Context, ERC165, IERC721, IERC721Metadata)
    - [Pub]  #
    - [Pub] supportsInterface
    - [Pub] balanceOf
    - [Pub] ownerOf
    - [Pub] name
    - [Pub] symbol
    - [Pub] tokenURI
    - [Int] _baseURI
    - [Pub] approve #
    - [Pub] getApproved
    - [Pub] setApprovalForAll #
    - [Pub] isApprovedForAll
    - [Pub] transferFrom #
    - [Pub] safeTransferFrom #
    - [Pub] safeTransferFrom #
    - [Int] _safeTransfer #
    - [Int] _exists
    - [Int] _isApprovedOrOwner
    - [Int] _safeMint #
    - [Int] _safeMint #
    - [Int] _mint #
    - [Int] _burn #
    - [Int] _transfer #
    - [Int] _approve #
    - [Int] _setApprovalForAll #
    - [Prv] _checkOnERC721Received #
    - [Int] _beforeTokenTransfer #
    - [Int] _afterTokenTransfer #

 +  ERC721Pausable (ERC721, Pausable)
    - [Int] _beforeTokenTransfer #

 +  ERC721URIStorage (ERC721)
    - [Pub] tokenURI
    - [Int] _setTokenURI #
    - [Int] _burn #

 + [Lib] Counters 
    - [Int] current
    - [Int] increment #
    - [Int] decrement #
    - [Int] reset #

 +  VLXNFT (ERC721URIStorage, AccessControl, Ownable)
    - [Pub]  #
       - modifiers: ERC721
    - [Pub] getMessageHash
    - [Pub] getEthSignedMessageHash
    - [Pub] verify
    - [Pub] recoverSigner
    - [Pub] splitSignature
    - [Pub] mint #
    - [Pub] getCreator
    - [Pub] burn #
    - [Int] _baseURI
    - [Pub] supportsInterface
    - [Pub] setTokenURI #
    - [Pub] approveBulk #
    - [Pub] getApprovedBulk

 +  ExchangeNFT (ERC721Holder, Ownable, Pausable)
    - [Pub]  #
    - [Ext] addCollection #
    - [Ext] getCollectionList
    - [Ext] setFeeAddress #
    - [Ext] setMarketingFee #
       - modifiers: onlyOwner
    - [Int] isCollectionExist
    - [Pub] getMessageHash
    - [Pub] getEthSignedMessageHash
    - [Pub] verify
    - [Pub] recoverSigner
    - [Pub] splitSignature
    - [Pub] compareStringsbyBytes
    - [Ext] transferToken #
    - [Ext] buyToken ($)
    - [Ext] sellToken #
    - [Ext] cancelOrder #
    - [Ext]  ($)

About SourceHat

SourceHat (formerly Solidity Finance - founded in 2020) has quickly grown to have one of the most experienced and well-equipped smart contract auditing teams in the industry. Our team has conducted 1700+ solidity smart contract audits covering all major project types and protocols, securing a total of over $50 billion U.S. dollars in on-chain value!
Our firm is well-reputed in the community and is trusted as a top smart contract auditing company for the review of solidity code, no matter how complex. Our team of experienced solidity smart contract auditors performs audits for tokens, NFTs, crowdsales, marketplaces, gambling games, financial protocols, and more!

What is a SourceHat Audit?

Typically, a smart contract audit is a comprehensive review process designed to discover logical errors, security vulnerabilities, and optimization opportunities within code. A SourceHat Audit takes this a step further by verifying economic logic to ensure the stability of smart contracts and highlighting privileged functionality to create a report that is easy to understand for developers and community members alike.

How Do I Interpret the Findings?

Each of our Findings will be labeled with a Severity level. We always recommend the team resolve High, Medium, and Low severity findings prior to deploying the code to the mainnet. Here is a breakdown on what each Severity level means for the project:

High severity indicates that the issue puts a large number of users' funds at risk and has a high probability of exploitation, or the smart contract contains serious logical issues which can prevent the code from operating as intended.
Medium severity issues are those which place at least some users' funds at risk and has a medium to high probability of exploitation.
Low severity issues have a relatively minor risk association; these issues have a low probability of occurring or may have a minimal impact.
Informational issues pose no immediate risk, but inform the project team of opportunities for gas optimizations and following smart contract security best practices.

Go Home

SourceHat

VLXNFTExchange

Smart Contract Audit Report

Audit Summary

Audit Findings

Finding #1 - ExchangeNFT - High (Resolved)

Finding #2 - VLXNFT - High (Resolved)

Finding #3 - VLXNFT - High (Resolved)

Finding #4 - ExchangeNFT - Medium (Resolved)

Finding #5 - ExchangeNFT - High (Resolved)

Finding #6 - ExchangeNFT - Low (Resolved)

Finding #7 - ExchangeNFT - Low (Resolved)

Finding #8 - ExchangeNFT - Low (Resolved)

Finding #9 - ExchangeNFT - Low (Resolved)

Finding #10 - VLXNFTExchange - Informational (Resolved)