Nym mixnet & vesting contracts security audit

Introduction

In December 2022, Nym underwent an independent security audit conducted by Oak Security, a Germany-based cybersecurity consulting firm specializing in auditing third-generation blockchains and decentralized protocols. With extensive experience in ecosystems such as Cosmos, Terra, Polkadot, and Flow, Oak Security was tasked with evaluating two critical components of the Nym ecosystem: the (1) the Nym mixnet and vesting contracts (see full report) and (2) the Nym Wallet (see full report). The audit aimed to assess the robustness of these components, identify potential vulnerabilities, and ensure adherence to best practices in code development. To make things easier to follow, we’ve broken down the findings for each component. You can check out the Nym Wallet audit summary here.

Summary of Nym Mixnet & Vesting Contracts Audit by Oak Security

Spanning 2 weeks, the audit involved a team of 4 experts. The Nym team provided Oak Security with full access to the project’s codebase, detailed project specifications, and supporting documentation. The scope of the audit covered the contracts/mixnet, contracts/vesting repositories, and relevant imports by these contracts for the Nym mixnet and vesting contracts.

Oak Security thoroughly reviewed the code, combining automated source code and dependency analysis with a manual line-by-line inspection to uncover security vulnerabilities, assess code quality, and evaluate compliance with secure coding principles. This comprehensive approach included an in-depth analysis of critical areas such as race condition vulnerabilities, underflow and overflow issues, key management practices, cryptographic robustness, data leakage risks, password handling, and access and authorization controls.

The focus of the audit was on ensuring the protocols' correct functionality, identifying any exploitable vulnerabilities or smart contract bugs, and recommending improvements for code safety and readability.

Overview of findings

The Nym mixnet and vesting contracts were characterized by high readability and clarity, with robust test coverage supporting their reliability. In their assessment, the auditors identified 19 findings, including 9 security vulnerabilities – comprising critical and major-severity issues – and 10 general weaknesses categorized as minor or informative.

The Nym team has swiftly addressed all critical and major issues. The Oak Security team verified and approved our fixes.

NYM-MIX-VEST-CONTRACT-1: A failing epoch or interval event execution permanently blocks epoch advancement (Critical)

The audit identified a vulnerability in the handling of AdvanceCurrentEpoch messages in contracts/mixnet/src/interval/transactions.rs. A sequential loop processes all pending events directly within the transaction context, meaning a single failing event could revert the entire transaction, effectively halting epoch and interval advancement and causing the protocol to become stuck.

To address this, the auditors recommended wrapping event execution in sub-messages with a reply policy. This approach would allow the system to handle failures gracefully without reverting the entire transaction. However, we deliberately chose not to follow this recommendation. A failure during event execution indicates a significant logic flaw that could compromise the reward mechanism, making it preferable to halt epoch advancement for manual inspection rather than risk operating in a corrupted state. Instead, we implemented enhanced monitoring in the Nym API to detect and alert us to failed epoch advancements. This approach ensures that such failures are promptly flagged, enabling timely intervention to resolve issues.

NYM-MIX-VEST-CONTRACT-2: Attackers can force orphan mix nodes to join a family without their consent to aggregate a large quantity of them in the same layer (Critical)

A vulnerability was identified in the JoinFamilyOnBehalf message, which could allow an attacker to add a mix node to a family without its consent. By creating a malicious family and using the victim mix node’s identity key signed with its private key as the signature, the attacker could trap the mix node in the family. This could potentially allow an attacker to group orphan mix nodes into a single family, disrupting the network by concentrating nodes in the same layer and increasing the attacker’s chances of influencing routing.

To address the vulnerability in the JoinFamilyOnBehalf message, the contract's signature mechanism was redesigned. Instead of simply signing the mix node's identity, signatures were updated to include a message with a clear intent and a unique nonce. This change ensures that signatures cannot be reused or replayed.

NYM-MIX-VEST-CONTRACT-3: Unbounded iteration in TrackUndelegation message handling could make the user unable to undelegate, which also permanently inhibits epoch advancement (Critical)

The TrackUndelegation message's unbounded iteration could cause gas exhaustion when handling accounts with numerous delegations, preventing users from undelegating. This failure would also halt epoch advancement, leaving the protocol stuck in its current state.

To address the issue, we implemented a limit of 25 vesting delegations per account. This solution was based on the highest observed number of vesting delegations at the time.

Update: As of 2024, the option to make vesting delegations is completely disabled.

NYM-MIX-VEST-CONTRACT-4: An attacker can front-run BondMixnodeOnBehalf and CreateFamilyOnBehalf messages and modify their payload (Critical)

This vulnerability would allow an attacker to frontrun BondMixNodeOnBehalf and CreateFamilyOnBehalf messages, extract the signature, and modify the payload. In the case of BondMixNodeOnBehalf, the attacker could also set themselves as the proxy, gaining full control over the registered bond.

To address the vulnerability, we implemented a twofold solution. First, we applied the signature creation changes described in NYM-MIX-VEST-CONTRACT-2 which added a clear intent and a unique nonce. Second, we restricted the legal proxy address to the vesting account, ensuring that attackers cannot set themselves as the proxy and gain unauthorized control over registered bonds.

Update: As of 2024 all proxy operations are disabled making the attack unapplicable.

NYM-MIX-VEST-CONTRACT-5: Signatures can be replayed within “on behalf” transactions to impersonate users (Critical)

This vulnerability allows signatures to be replayed within “on behalf” transactions, enabling attackers to impersonate users. Since the signatures only include raw data without additional metadata like nonces or message identifiers, an attacker could reuse a signature from one message for another to forcibly remove family members. Additionally, attackers can reuse signatures for the same message type, allowing a member to rejoin a family multiple times using the same signature.

The solution to this issue mirrors the one implemented in NYM-MIX-VEST-CONTRACT-4.

NYM-MIX-VEST-CONTRACT-6: Key generation for (owner, proxy) pairs could lead to collisions (Critical)

This vulnerability would allow two different (owner, proxy) address pairs to generate the same XOR result in the generate_owner_storage_subkey function, which could lead to key collisions and data overwrites.

The solution of restricting the legal proxy to only the vesting contract address, which was applied in the NYM-MIX-VEST-CONTRACT-4, also solves this vulnerability. By ensuring that the XOR operation is always performed against a constant string, this change effectively eliminates the risk of collisions.

NYM-MIX-VEST-CONTRACT-7: track_delegation can overwrite existing delegation if more than one is processed within the same block (Critical)

This issue would allow key collisions to occur if multiple delegations are made within the same block, potentially causing the save_delegation function to overwrite an existing delegation, resulting in the loss of the previous delegation.

To address the issue, we changed the logic of the save_delegation function to first read the existing delegation amount before saving the new one. If a delegation already exists for the same key, we retrieve the current amount, add the new delegation, and then save the updated total, ensuring no existing delegation is overwritten.

NYM-MIX-VEST-CONTRACT-8: Bond owner is not able to perform operations if the bond is created “on behalf” by a proxy (Major)

This issue would prevent the bond owner from performing any operations on their bond if the bond was created "on behalf" and the current transaction is initiated by the owner instead of the proxy. Since the proxy could be compromised or lost, and there is no way for the owner to change it, this could result in the owner losing control over their bond.

We do not consider this to be an issue, as it is intentional behavior by design. When a mix node is bonded (or delegations are made) through the vesting contract, all subsequent interactions with the bond or delegation are expected to occur only via the vesting contract.

Update: As of 2024, the vesting contract is removed.

NYM-MIX-VEST-CONTRACT-9: LeaveFamily and LeaveFamilyOnBehalf messages execution could fail if a significant amount of mix nodes are members of a family (Major)

This issue would result in the failure of LeaveFamily and LeaveFamilyOnBehalf messages due to gas exhaustion when iterating over a large number of family members in the MEMBERS map. This would prevent mix nodes from leaving a family, effectively locking them in their current family.

Solution: The solution involved reworking the logic to directly access the MEMBERS map and verify the membership of a mix node. This change removed the need for iteration, which could result in out-of-gas errors, and ensured that members can leave a family successfully, even when there are many members in the family.

Update: As of 2024, node families are removed.

Last words

We would like to thank the Oak Security team for their expertise and dedication throughout this audit process. We also appreciate the collaboration and professionalism demonstrated during both the planning and execution stages of the audit. Our ongoing commitment to security remains a top priority, and we look forward to continued partnerships with security experts to uphold the highest standards for our ecosystem.