Nya logo

Nya

Audit name:

[SCA] Nya / Nya Exchange / Sept2024

Date:

Oct 10, 2024

Table of Content

→

Introduction

→

Audit Summary

→

System Overview

→

Potential Risks

→

Findings

→

Appendix 1. Definitions

→

Appendix 2. Scope

→

Appendix 3. Additional Valuables

→

Disclaimer

Introduction

We express our gratitude to the Nya team for the collaborative engagement that enabled the execution of this Smart Contract Security Assessment.

The Nya Exchange Marketplace is a decentralized exchange that facilitates efficient trading of digital assets within the Catgirl Token ecosystem.

  • Document

    Name
    Smart Contract Code Review and Security Analysis Report for Nya
    Audited By
    Ivan Bondar, Viktor Lavrenenko
    Approved By
    Przemyslaw Swiatowiec
    Website
    https://www.nya.vip/→
    Changelog
    25/09/2024 - Preliminary Report
    11/10/2024 - Final Report
    Platform
    BSC, ETH, BASE, ARB, OP, AVAX, POLYGON
    Language
    Solidity
    Tags
    Marketplace; Auction; Signatures; Proxy; Centralization; Upgradable
    Methodology
    https://hackenio.cc/sc_methodology→

Audit Summary

17Total Findings
15Resolved
2Accepted
0Mitigated

The system users should acknowledge all the risks summed up in the risks section of the report

Documentation quality

  • Functional requirements are described.

    • Project overview is detailed.

    • All roles in the system are described.

    • Use cases are described and detailed.

    • For each contract, all features are described.

  • Technical description is limited.

    • Run instructions are not provided.

    • Technical specification is provided.

    • The NatSpec documentation is sufficient.

Code quality

  • The development environment is configured.

  • Solidity Style Guide violations are present.

Test coverage

Code coverage of the project is 58.33% (branch coverage).

  • Deployment and basic user interactions are covered with tests.

  • Test cases for negative scenarios is partially missed

  • Interactions by several users are not tested thoroughly.

  • No tests currently verify the actual fees received.

  • No tests ensure that ERC20 tokens are correctly paid for NFTs.

System Overview

The Nya Exchange module provides a decentralized platform for buying and selling digital assets within the Catgirl Token ecosystem. It enables users to create, approve, and fulfill orders for NFTs and other tokens using a secure and efficient exchange protocol. The marketplace supports various sale types, including fixed price and Dutch auctions, and allows for transactions using both Ether and ERC20 tokens.

The system leverages a proxy architecture to manage user assets. Each user is associated with an AuthenticatedProxy contract, which acts on their behalf to execute trades and interact with other contracts. The ProxyRegistry maintains a mapping of users to their proxies and controls which contracts are authorized to interact with these proxies.

Key features of the marketplace include configurable maker and taker fees, support for upgradeable contracts through the UUPS proxy pattern, and the ability to handle complex trade executions with minimal overhead.

Files in Scope:

  • NyaExchange.sol: The main exchange contract that users interact with. It implements the core trading logic, including order matching, execution, and fee handling.

  • ProxyRegistry.sol: Manages user proxies (OwnableDelegateProxy instances) and maintains a list of authorized contracts that can interact with these proxies.

  • OwnableDelegateProxy.sol: A proxy contract owned by a user that delegates calls to an implementation contract (AuthenticatedProxy), enabling upgradeability and ownership control.

  • AuthenticatedProxy.sol: Executes calls on behalf of the user, allowing for actions like transferring tokens or executing trades. It ensures only authorized contracts can interact with user assets.

  • OwnedUpgradeabilityProxy.sol and OwnedUpgradeabilityStorage.sol: Implement upgradeable proxy functionality, allowing contracts to be upgraded without changing their address.

  • ExchangeCore.sol: Contains the core functionality for order handling, including hashing, validation, and execution of orders.

  • Exchange.sol: Extends ExchangeCore and provides additional public functions for order management and matching.

  • ArrayUtils.sol: Utility library for array manipulation, used for handling calldata replacements and comparisons.

  • SaleKindInterface.sol: Defines the types of sales (fixed price, Dutch auction) and contains functions related to sale validation and pricing.

  • TokenRecipient.sol: Handles receiving tokens and Ether, emitting events for such actions.

Privileged roles

NyaExchange.sol:

  • DEFAULTADMINROLE: Has full administrative control over the contract, including assigning other roles.

  • UPGRADER_ROLE: Can upgrade the contract implementation via the UUPS upgradeable proxy pattern.

  • SETTER_ROLE: Can update critical configuration parameters such as maker and taker fees, proxy registry address, and the Catgirl coin address.

ProxyRegistry.sol:

  • Owner: Can grant or revoke authentication to contracts, controlling which contracts are allowed to interact with user proxies.

AuthenticatedProxy.sol:

  • User: The owner of the proxy who can set or revoke access, effectively controlling which contracts can execute actions on their behalf.

  • Registry Contracts: Contracts authorized by the ProxyRegistry. They can interact with the proxy unless the user has revoked access.

OwnedUpgradeabilityProxy.sol:

  • Upgradeability Owner: Has the authority to upgrade the implementation address of the proxy, allowing for contract logic to be updated.

Potential Risks

Scope Definition and Security Guarantees: The audit does not cover all code in the repository. Contracts outside the audit scope may introduce vulnerabilities, potentially impacting the overall security due to the interconnected nature of smart contracts.

Single Points of Failure and Control: The project is fully or partially centralized, introducing single points of failure and control. This centralization can lead to vulnerabilities in decision-making and operational processes, making the system more susceptible to targeted attacks or manipulation.

Administrative Key Control Risks: The digital contract architecture relies on administrative keys for critical operations. Centralized control over these keys presents a significant security risk, as compromise or misuse can lead to unauthorized actions or loss of funds.

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.

Absence of Upgrade Window Constraints: The contract suite allows for immediate upgrades without a mandatory review or waiting period, increasing the risk of rapid deployment of malicious or flawed code, potentially compromising the system's integrity and user assets.

Solidity Version Compatibility and Cross-Chain Deployment: The project utilizes Solidity version 0.8.20 or higher, which introduces the PUSH0 (0x5f) opcode. While supported on Ethereum mainnet, this opcode may not be compatible with certain Layer 2 chains or alternative blockchains. Ensuring compatibility across multiple chains is crucial. Tools like EVMdiff → can be used to compare opcode support across chains to avoid potential deployment failures or execution errors on networks that do not support this opcode.

Off-Chain Data Handling: Reliance on off-chain processes for salt and order creation introduces potential points of failure or manipulation, which could compromise order validity and security.

Signature and Data Integrity: Users must exercise caution when creating and signing orders to ensure the accuracy and appropriateness of the data being signed. Signed orders remain valid until their specified expiration, and failure to cancel outdated or unfavorable orders can result in their exploitation at inappropriate prices or conditions.

Order Validity and On-Chain Ownership Verification: The protocol relies on off-chain validation to verify NFT ownership and manage order states. If users transfer NFTs between wallets and subsequently withdraw them without canceling their orders, active listings may remain valid at outdated or unfavorable prices. This persistence can lead to potential exploitation if ownership changes are not promptly reflected on-chain, necessitating frontend measures to mitigate such risks.

Frontrunning During The Initialization Of The Implementation Contract: If the deployment and initialization of the implementation contract (NyaExchange.sol) are not performed within a single transaction, there is a risk that another entity could frontrun the NyaExchange::initialize() function.

Report Modification: This report was modified on October 28th, 2024, at the client’s request to update its original content by updating the client entity name. While these changes aim to align the report with the most current information provided by the client, it is important to note that modifying previously published content may affect the integrity and continuity of the original audit findings. Hacken has reviewed the modifications to confirm they reflect only the requested updates, but any future changes involving substantial updates or new code commits should be accompanied by a re-assessment to ensure no new risks compromise the security posture.

Findings

Code
―
Title
Status
Severity
F-2024-6214
Misdirected Maker Relayer Fee in executeFundsTransfer Burdens Seller with Unintended Fees
Fixed

Medium
F-2024-6124
Order Execution Failure for Native Currency Transactions Due to Use of .send
Fixed

Medium
F-2024-6123
Incompatibility with Non-Standard ERC20 Tokens (e.g., USDT) Leads to Order Execution Failure
Fixed

Medium
F-2024-6188
Missing Storage Gaps Can Lead to Storage Collision in Upgradeable Contracts
Fixed

Low
F-2024-6178
Lack of Delay in startGrantAuthentication in ProxyRegistry.sol May Lead to Unauthorized Access
Fixed

Low
F-2024-6212
Token Transfer in executeFundsTransfer Occurs After Fee Deductions, Contradicting Intended Logic
Fixed

Observation
F-2024-6192
Unnecessary Initialization of Variables to Default Values
Fixed

Observation
F-2024-6190
Missing Revert Messages in require Statements
Fixed

Observation
F-2024-6171
Manual Order Struct Serialization Using Inline Assembly
Accepted

Observation
F-2024-6166
Unused sizeOf Function Remains in Codebase
Fixed

Observation
1-10 of 17 findings

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

    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, do not affect security score but can affect code quality score.

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:

Contracts in Scope

contracts
NyaExchange.sol - contracts/NyaExchange.sol
marketplace
registry
ProxyRegistry.sol - contracts/marketplace/registry/ProxyRegistry.sol
OwnableDelegateProxy.sol - contracts/marketplace/registry/OwnableDelegateProxy.sol
AuthenticatedProxy.sol - contracts/marketplace/registry/AuthenticatedProxy.sol
proxy
OwnedUpgradeabilityProxy.sol - contracts/marketplace/registry/proxy/OwnedUpgradeabilityProxy.sol
OwnedUpgradeabilityStorage.sol - contracts/marketplace/registry/proxy/OwnedUpgradeabilityStorage.sol
Proxy.sol - contracts/marketplace/registry/proxy/Proxy.sol
exchange
ExchangeCore.sol - contracts/marketplace/exchange/ExchangeCore.sol
Exchange.sol - contracts/marketplace/exchange/Exchange.sol
libraries
TokenRecipient.sol - contracts/marketplace/libraries/TokenRecipient.sol
SaleKindInterface.sol - contracts/marketplace/libraries/SaleKindInterface.sol
ArrayUtils.sol - contracts/marketplace/libraries/ArrayUtils.sol

Appendix 3. Additional Valuables

Verification of System Invariants

During the audit of  NYA Exchange, Hacken followed its methodology by performing fuzz-testing on the project's main functions. Echidna →, a tool used for fuzz-testing, was employed to check how the protocol behaves under various inputs. Due to the complex and dynamic interactions within the protocol, unexpected edge cases might arise. Therefore, it was important to use fuzz-testing to ensure that several system invariants hold true in all situations.

Fuzz-testing allows the input of many random data points into the system, helping to identify issues that regular testing might miss. A specific Echidna fuzzing suite was prepared for this task, and throughout the assessment, 10 invariants were tested over 5,000,000 runs. This thorough testing ensured that the system works correctly even with unexpected or unusual inputs.

  • Invariant

    Exchange::orderCalldataCanMatch() should always return true for the same calldata specifications

    Test Result

    Passed

    Run Count

    5M

    Invariant

    Exchange::orderCalldataCanMatch() should always return false for different calldata specifications

    Test Result

    Passed

    Run Count

    5M

    Invariant

    NyaExchange::setMakerCatgirlFee() should always revert for makerCatgirlFee greater than MAXIMUM_RELAYER_FEE

    Test Result

    Passed

    Run Count

    5M

    Invariant

    NyaExchange::setTakerCatgirlFee() should always revert for takerCatgirlFee greater than MAXIMUM_RELAYER_FEE

    Test Result

    Passed

    Run Count

    5M

    Invariant

    NyaExchange::setMakerFee() should always revert for makerFee greater than MAXIMUM_RELAYER_FEE

    Test Result

    Passed

    Run Count

    5M

    Invariant

    NyaExchange::setTakerFee() should always revert for takerFee greater than MAXIMUM_RELAYER_FEE

    Test Result

    Passed

    Run Count

    5M

    Invariant

    NyaExchange::setMakerCatgirlFee() does not revert for reasonable makerCatgirlFee

    Test Result

    Passed

    Run Count

    5M

    Invariant

    NyaExchange::setTakerCatgirlFee() does not revert for reasonable takerCatgirlFee

    Test Result

    Passed

    Run Count

    5M

    Invariant

    NyaExchange::setMakerFee() does not revert for reasonable makerFee

    Test Result

    Passed

    Run Count

    5M

    Invariant

    NyaExchange::setTakerFee() does not revert for reasonable takerFee

    Test Result

    Passed

    Run Count

    5M

Disclaimer