- Hacken
- Audits
- snag-solutions
- [SCA] Snag Solutions | MarketHybrid | Nov2024
Audit name:
[SCA] Snag Solutions | MarketHybrid | Nov2024
Date:
Nov 29, 2024
Table of Content
Introduction
We express our gratitude to the Snag Solutions team for the collaborative engagement that enabled the execution of this Smart Contract Security Assessment.
The Snag web application is a white-label NFT marketplace and loyalty solution which is a multi-tenant system.
Document | |
|---|---|
| Name | Smart Contract Code Review and Security Analysis Report for Snag Solutions |
| Audited By | Seher Saylik, Giovanni Franchi |
| Approved By | Ataberk Yavuzer |
| Website | https://www.snagsolutions.io/→ |
| Changelog | 06/11/2024 - Preliminary Report |
| Changelog | 25/11/2024 - Final Report |
| Platform | Base, ApeChain/Xai (arb l3s), Polygon, ZKSync |
| Language | Solidity |
| Tags | MarketPlace, ERC-721 |
| Methodology | https://hackenio.cc/sc_methodology→ |
Document
- Name
- Smart Contract Code Review and Security Analysis Report for Snag Solutions
- Audited By
- Seher Saylik, Giovanni Franchi
- Approved By
- Ataberk Yavuzer
- Changelog
- 06/11/2024 - Preliminary Report
- Changelog
- 25/11/2024 - Final Report
- Platform
- Base, ApeChain/Xai (arb l3s), Polygon, ZKSync
- Language
- Solidity
- Tags
- MarketPlace, ERC-721
- Methodology
- https://hackenio.cc/sc_methodology→
Review Scope | |
|---|---|
| Repository | https://github.com/Snag-Solutions/contracts→ |
| Commit | 148512c |
Review Scope
- Commit
- 148512c
Audit Summary
The system users should acknowledge all the risks summed up in the risks section of the report
Documentation quality
Functional requirements are mostly provided.
Technical description is not provided.
NatSpecs are provided and sufficient.
Code quality
The code violates best practises in terms of gas efficiency.
The code presents some misalignment in common best practises related to contract ownership and proxy layout.
The development environment is configured.
Test coverage
Code coverage of the project is 37.5% (branch coverage).
Tests cover the primary functions including deployment, signature generation, and calling the
purchaseItemsfunction.The coverage for edge cases such as invalid signatures, expired deadlines, non-existent
orgIdkeys etc. is missing.
System Overview
SnagMarketHybridV2 — It is a white-label market place with multiple tenants. Each tenant represent a Snag client and a market owner. The intent of the system is to facilitate NFT purchase with signatures and off-chain loyalty payments.
Privileged roles
The owner can add, remove a modify signing keys related to a specific operator id.
The owner can cancel orders.
Potential Risks
The protocol flow is partially managed off-chain, specifically the points payment process, which is handled entirely off-chain. It is important to highlight that this audit cannot guarantee the correctness of these off-chain operations. Therefore, users are advised to carefully consider this aspect before deciding to interact with the protocol.
Flexibility and Risk in Contract Upgrades: The project's contracts are upgradable, allowing the administrator to update the contract logic at any time. While this provides flexibility in addressing issues and evolving the project, it also introduces risks if upgrade processes are not properly managed or secured, potentially allowing for unauthorized changes that could compromise the project's integrity and security.
Findings
Code ― | Title | Status | Severity | |
|---|---|---|---|---|
| F-2024-6957 | Inadequate Validation of fromAddress in purchaseItems Function | fixed | High | |
| F-2024-6960 | Solidity Version 0.8.20 Might Not Work On All Chains Due To PUSH0 | accepted | Low | |
| F-2024-6953 | Missing Storage Gaps for Upgradeable Contracts | fixed | Low | |
| F-2024-6945 | Violation of Checks-Effects-Interactions (CEI) Pattern | accepted | Low | |
| F-2024-6944 | Risk of Hash Collisions from Using abi.encodePacked in Array Hashing | accepted | Low | |
| F-2024-6964 | Unneeded Initializations Of Integer Variable To 0 | accepted | Observation | |
| F-2024-6962 | Use private Rather than public for Constants | accepted | Observation | |
| F-2024-6961 | Contract Uses Both require()/revert() As Well As Custom Errors | accepted | Observation | |
| F-2024-6951 | Custom errors can be used to improve gas efficiency | accepted | Observation | |
| F-2024-6950 | Missing disableInitializers() Can Lead to Unwanted Initialization | accepted | Observation |
Appendix 1. Definitions
Severities
When auditing smart contracts, Hacken is using a risk-based approach that considers Likelihood, Impact, Exploitability and Complexity metrics to evaluate findings and score severities.
Reference on how risk scoring is done is available through the repository in our Github organization:
Severity | Description |
|---|---|
Critical | Critical vulnerabilities are usually straightforward to exploit and can lead to the loss of user funds or contract state manipulation. |
High | High vulnerabilities are usually harder to exploit, requiring specific conditions, or have a more limited scope, but can still lead to the loss of user funds or contract state manipulation. |
Medium | Medium vulnerabilities are usually limited to state manipulations and, in most cases, cannot lead to asset loss. Contradictions and requirements violations. Major deviations from best practices are also in this category. |
Low | Major deviations from best practices or major Gas inefficiency. These issues will not have a significant impact on code execution. |
Severity
- Critical
Description
- Critical vulnerabilities are usually straightforward to exploit and can lead to the loss of user funds or contract state manipulation.
Severity
- High
Description
- High vulnerabilities are usually harder to exploit, requiring specific conditions, or have a more limited scope, but can still lead to the loss of user funds or contract state manipulation.
Severity
- Medium
Description
- Medium vulnerabilities are usually limited to state manipulations and, in most cases, cannot lead to asset loss. Contradictions and requirements violations. Major deviations from best practices are also in this category.
Severity
- Low
Description
- Major deviations from best practices or major Gas inefficiency. These issues will not have a significant impact on code execution.
Potential Risks
The "Potential Risks" section identifies issues that are not direct security vulnerabilities but could still affect the project’s performance, reliability, or user trust. These risks arise from design choices, architectural decisions, or operational practices that, while not immediately exploitable, may lead to problems under certain conditions. Additionally, potential risks can impact the quality of the audit itself, as they may involve external factors or components beyond the scope of the audit, leading to incomplete assessments or oversight of key areas. This section aims to provide a broader perspective on factors that could affect the project's long-term security, functionality, and the comprehensiveness of the audit findings.
Appendix 2. Scope
The scope of the project includes the following smart contracts from the provided repository:
Scope Details | |
|---|---|
| Repository | https://github.com/Snag-Solutions/contracts→ |
| Commit | 148512ce64a3e6748ca7d81fdcdc143da1e5f977 |
| Whitepaper | N/A |
| Requirements | ./docs/V2AuditContext.md |
| Technical Requirements | N/A |
Scope Details
- Commit
- 148512ce64a3e6748ca7d81fdcdc143da1e5f977
- Whitepaper
- N/A
- Requirements
- ./docs/V2AuditContext.md
- Technical Requirements
- N/A
Assets in Scope
Appendix 3. Additional Valuables
Additional Recommendations
The smart contracts in the scope of this audit could benefit from the introduction of automatic emergency actions for critical activities, such as unauthorized operations like ownership changes or proxy upgrades, as well as unexpected fund manipulations, including large withdrawals or minting events. Adding such mechanisms would enable the protocol to react automatically to unusual activity, ensuring that the contract remains secure and functions as intended.
To improve functionality, these emergency actions could be designed to trigger under specific conditions, such as:
Detecting changes to ownership or critical permissions.
Monitoring large or unexpected transactions and minting events.
Pausing operations when irregularities are identified.
These enhancements would provide an added layer of security, making the contract more robust and better equipped to handle unexpected situations while maintaining smooth operations.