Inside the Fix: QuillAudits' Strategy for Securing Virtualness Protocol

1. Information Gathering
   • Collected and reviewed all relevant documentation, including whitepaper, technical specifications, and design documents.
   • Obtained a clear understanding of the Virtualness platform's functionality and intended user interactions.
   • Discussed client concerns and specific areas of focus for the audit.
2. Manual Code Review:
   • Conducted a line-by-line review of the smart contract code, focusing on:
     • Vulnerability identification: Searching for known vulnerabilities like reentrancy, front-running, integer overflows, and access control issues, etc.
     • Logic flaws: Identifying inconsistencies or unintended behaviours in the code logic.
     • Solidity best practices: Compliance with secure coding standards and adherence to established guidelines.
3. Functional Testing:
   • Developed and executed a comprehensive set of test cases covering various user interactions and edge cases.
   • Leveraged tools like Hardhat and Ganache to deploy and test the smart contract locally.
4. Automated Testing:
   • Employed static analysis tools like QuillShield to identify vulnerabilities through automated code scanning.
   • Utilized symbolic execution tools like Mythril to explore various code execution paths and uncover potential attack vectors.
   • Integrated unit tests are written by the Virtualness team to verify specific contract functions and their behaviour.
5. Reporting & Remediation:
   • Prepared a detailed report outlining all identified vulnerabilities, categorized by severity and potential impact.
   • Provided clear recommendations for fixing each vulnerability, including code snippets and best practices.
   • Collaborated with the Virtualness Protocol team to prioritize and address the identified issues.
   • Conducted additional verification testing after vulnerability fixes were implemented.

Reuse of MintData for Different Orders

• Discovery: MintData intended for one order could be incorrectly used for another if conditions such as sell.order.trader matching mintData.creators[0].account and sell.order.collection.tokenId matching mintData.tokenId are met. This loophole didn't thoroughly check other details like URI and supply during execution.
• Impact: This could result in unintended token swaps where the wrong MintData gets applied, causing discrepancies in token characteristics and unexpected transactions.

2. Royalty Fee Transfer Issue during Lazy Minting

• Discovery: In the fulfillLazyMintOrder function, there was an issue during lazy minting where addresses meant to receive royalty fees weren't always retrieved correctly after transferring funds with transferFunds(). This sometimes led to situations where royalty fees ended up not reaching their intended recipients, often due to incorrect or zero addresses being retrieved after the transfer.
• Impact: This issue could disrupt how royalty fees are distributed for NFTs minted through lazy minting, which could affect revenue and incentives for users.

3. Seller Order Manipulation Before Signing

• Discovery: Backend-added platform fees within seller orders were vulnerable to manipulation by malicious signers before the orders were officially signed. This vulnerability allowed unauthorized changes to fee structures and other order details.
• Impact: It posed risks of financial losses and compromised trust in transactions because unauthorized alterations could skew platform fees and compromise the integrity of the orders.

4. Lack of SafeERC Usage for Critical Operations

• Discovery: Within functions like transferFunds() and transferERC20(), there were lapses in checking critical details during token transfers. For instance, after executing transferFrom(), the system didn’t consistently verify the boolean status. This oversight could potentially allow malicious buyers to acquire tokens without actually spending any.
• Impact: It exposed the platform to risks of token loss and exploitation, which could undermine user confidence and overall platform security.

5. Address Role Removal Considerations

• Discovery: Issues surfaced during lazy minting lazyMint(), where revoked minter roles for those signing MintData could cause transactions to fail. This happened when individuals with revoked roles attempted to execute transactions related to MintData, resulting in transaction reversals.
• Impact: Such occurrences could disrupt operations and lead to failed transactions during the NFT minting process, especially when the system didn’t handle removed minter roles properly.

• Reuse of MintData for Different Orders: QuillAudits enhanced the fulfillOrder function with proper validation checks. This ensured that MintData was exclusively used for its intended order, preventing any kind of unauthorized substitutions.
• Royalty Fee Transfer Issue during Lazy Minting: To resolve this, QuillAudits reorganized operations within fulfillLazyMintOrder. This adjustment ensured that recipient addresses were accurately retrieved before fund transfers, thereby preventing instances where royalty fees didn’t reach their intended recipients.
• Seller Order Manipulation Before Signing: QuillAudits worked with Virtualness Protocol’s team to refine the signOrder function. By implementing cryptographic validations, they secured platform fees and order parameters post-signing, ensuring they couldn’t be tampered with.
• Lack of SafeERC Usage for Critical Operations: QuillAudits adopted SafeERC standards across functions like transferFunds() and transferERC20(). This involved implementing robust checks to verify token transfers, thus reducing risks associated with token loss and exploitation.
• Address Role Removal Considerations: QuillAudits recommended improvements in the logic of the lazyMint() function. This included adding checks to handle scenarios where minter roles were revoked, ensuring that MintData-related transactions continued smoothly without encountering failures due to role removal.

During the audit, several functional tests were conducted to ensure the robustness and correctness of the Virtualness protocol smart contracts.

Here are some of the critical tests performed:

ProtocolV1Core Tests:

1. Opening and Closing:
   • The owner should be able to open and close the protocol.
2. Token Exchange:
   • Users should successfully use fulfillOrder for selling and buying already minted tokens.
   • fulfillLazyMintOrder should work for selling and buying through lazy minting.
3. Order Cancellation:
   • Sellers should be able to cancel their orders.
   • Incrementing the counter to cancel all orders by changing the nonce should function as expected.
4. Order Validation:
   • Reverting if the buyer tries to buy more than what the seller wants to sell.
   • Handling order expiration time correctly.
   • Reverting if the sell signature is invalid during signer recovery.

ERC721Virtualness Tests:

1. Minting Tokens:
   • The contract should allow minting of ERC721 tokens.
2. Delegated Minting:
   • Delegated minting (by adding minter role) should work as intended.
3. Lazy Minting:
   • Lazy minting of ERC721 tokens should be successful.
4. Minter Management:
   • The owner should be able to add and remove minters.

ERC1155Virtualness Tests:

1. Minting Tokens:
   • ERC1155 tokens should be minted correctly.
2. Delegated Minting:
   • Delegated minting (by adding minter role) for ERC1155 tokens should function properly.
3. Lazy Minting:
   • Lazy minting of ERC1155 tokens should work as expected.
4. Supply Constraints:
   • Reverting if an attempt is made to mint more than the defined supply.
5. Minter Management:
   • The owner should have control over adding and removing minters.