NEST
Smart Contract Audit Report
Audit Summary
NEST is building a series of smart contracts to enable multi-sig functionality, identity verification, an NFT locking mechanism, and a new NFT.
For this audit, we reviewed the project team's MultiSigWallet, NESTDIDRegistry, TOFToken, and NestLocker contracts at commit 99051688f9dec6c0897ec7321f88ce8047abe46e on the team's private GitHub repository.
We previously reviewed the project team's token contract here.
Audit Findings
High findings were identified and the team must resolve these issues.
Date: June 30th, 2022.Finding #1 - TOFToken, NestLocker - High
Description: The TOFToken contract has implemented the onERC721Received() function and the NestLocker contract has implemented the onERC1155Received() and onERC1155BatchReceived() functions which allow the contracts to receive these types of NFTs. However, there is no way to withdraw these NFTs from the contract.
Risk/Impact: As there is no withdraw mechanism for these types of NFTs, any NFTs received by these contracts will be permanently locked.
Recommendation: The team should implement a function to withdraw any NFTs in the contracts.
Resolution: The team has not yet addressed this issue.Finding #2 - MultiSigWallet - Medium
Description: Owners can only be added to an SSID; they cannot be removed. In addition, the team should notice that the addOwners() function can be called multiple times and the array stored in the ssidOwners mapping at the desired index will be overwritten on each call.
Risk/Impact: If an address is added erroneously as an owner for a given SSID, they will have access to critical functions such as changing the minimum requirement for executing transactions, adding whitelisters, submitting transactions, confirming transactions, and executing transactions.
Recommendation: The team should ensure that owners can be appropriately disassociated with a given SSID.
Resolution: The team has not yet addressed this issue.Finding #3 - TOFToken - Informational
Description: The transferNFT() function is redundant as it is the same as the transferFrom() function provided by the inherited ERC-721 contract.
Recommendation: The team can remove this function for additional savings on deployment costs.
Contracts Overview
MultiSigWallet Contract:
- As the contracts are implemented with Solidity v0.8.x, they are safe from any possible overflows/underflows.
NESTDIDRegistry Contract:
- This contract is used to facilitate a multi-sig wallet mechanism in which groups of owners can mark specific transactions as executed once a required number of confirmations is met.
- The creator address can associate a list of owners with a specific SSID at any time, specifying the number of confirmations required in order for transactions to be marked as executed.
- Any owner for a given SSID can change the minimum confirmation requirement at any time.
- Any owner for a given SSID can submit a transaction at any time.
- Any owner for a given SSID can provide a confirmation for a submitted transaction until the required amount of confirmations has been met.
- Once the required amount of confirmations has been met, the transaction is marked as executed.
- Any owner for a given SSID can associate a list of whitelisted addresses with a specific SSID at any time.
TOFToken Contract:
- This contract allows the contract owner to associate an identity string with a wallet or contract address.
- The contract owner can change the identity string associated with any wallet or contract address at any time.
- The contract includes logic that can be used to verify that a hashed message provided by the user containing the identity string and the associated wallet address has been signed by the associated wallet address.
NestLocker Contract:
- This contract represents the team's Real World Smart Contract Token NFT ($RWSCT).
- The owner can mint any amount of NFTs to any address at any time.
- Any NFT holder, or anyone with an approval for the NFT, can set the token URI associated with the NFT ID at any time.
- The contract complies with the ERC-721 standard.
- This contract is used receive various ERC-1155 tokens through any available means.
- There is no way to withdraw any ERC-1155 NFTs from the contract.
Audit Results
| Vulnerability Category | Notes | Result |
|---|---|---|
| Arbitrary Jump/Storage Write | N/A | PASS |
| Centralization of Control | N/A | PASS |
| 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 |
| WARNING |
| 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 | WARNING |
MultiSigWallet Contract
($) = payable function
# = non-constant function
Int = Internal
Ext = External
Pub = Public
+ MultiSigWallet
- [Pub] #
- [Pub] addOwners #
- modifiers: onlyCreator,addOwnerRequirement
- [Pub] addWhitelisters #
- modifiers: onlyWallet
- [Pub] changeRequirement #
- modifiers: onlyWallet,validRequirement
- [Pub] submitTransaction #
- modifiers: onlyWallet
- [Pub] confirmTransaction #
- modifiers: accessToConfirm,notConfirmed
- [Int] executeTransaction #
- modifiers: notExecuted
- [Pub] isConfirmed
- [Int] addTransaction #
- [Pub] getConfirmationCount
- [Pub] getTransactionCount
- [Pub] getOwners
- [Pub] getWhiteListers
- [Pub] getConfirmations
- modifiers: transactionExists NESTDIDRegistry Contract
($) = payable function
# = non-constant function
Int = Internal
Ext = External
Pub = Public
+ NestDIDRegistry
- [Pub] #
- [Pub] identityOwner
- [Pub] addIdentity #
- modifiers: onlyOwner
- [Pub] changeOwner #
- modifiers: onlyOwner
- [Pub] verifySigner
- [Int] recoverSigner
- [Int] splitSignature TOFToken Contract
($) = payable function
# = non-constant function
Int = Internal
Ext = External
Pub = Public
+ [Int] IERC165
- [Ext] supportsInterface
+ [Int] IERC721 (IERC165)
- [Ext] balanceOf
- [Ext] ownerOf
- [Ext] safeTransferFrom #
- [Ext] safeTransferFrom #
- [Ext] transferFrom #
- [Ext] approve #
- [Ext] setApprovalForAll #
- [Ext] getApproved
- [Ext] isApprovedForAll
+ [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
+ Context
- [Int] _msgSender
- [Int] _msgData
+ [Lib] Strings
- [Int] toString
- [Int] toHexString
- [Int] toHexString
+ ERC165 (IERC165)
- [Pub] supportsInterface
+ 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 #
+ ERC721URIStorage (ERC721)
- [Pub] tokenURI
- [Int] _setTokenURI #
- [Int] _burn #
+ TOFToken (ERC721URIStorage)
- [Pub] #
- modifiers: ERC721
- [Pub] mintNFT #
- modifiers: onlyOwner
- [Pub] setTokenURI #
- [Pub] transferNFT #
- [Pub] onERC721Received NestLocker Contract
($) = payable function
# = non-constant function
Int = Internal
Ext = External
Pub = Public
+ [Int] IERC165
- [Ext] supportsInterface
+ [Int] IERC1155Receiver (IERC165)
- [Ext] onERC1155Received #
- [Ext] onERC1155BatchReceived #
+ ERC165 (IERC165)
- [Pub] supportsInterface
+ ERC1155Receiver (ERC165, IERC1155Receiver)
- [Pub] supportsInterface
+ ERC1155Holder (ERC1155Receiver)
- [Pub] onERC1155Received #
- [Pub] onERC1155BatchReceived #
+ [Int] IERC721Receiver
- [Ext] onERC721Received #
+ NestLocker (ERC1155Holder)About SourceHat
SourceHat has quickly grown to have one of the most experienced and well-equipped smart contract auditing teams in the industry. Our team has conducted 1300+ 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 across 1500 projects!.
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!
Contact us today to get a free quote for a smart contract audit of your project!
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.