Pluribit Codebase Audit: Bugs, Issues, & Improvements
Welcome to a comprehensive analysis of the Pluribit codebase. This audit uncovers a variety of issues, ranging from critical bugs to areas for improvement, to provide a clear picture of the project's current state. The analysis is based on the provided codebase and aims to identify potential risks, vulnerabilities, and inefficiencies. Let's dive in and examine the findings.
🔴 Critical Issues
These are the most pressing concerns that need immediate attention. They pose significant risks to the functionality, security, and stability of the Pluribit project.
1. Unimplemented Functions in lib.rs (lines 617-727)
Many handler functions are currently stubbed out, meaning they lack proper implementation. These functions, such as handle_create_transaction_internal, handle_toggle_miner_internal, and others, are critical for core operations. The use of log("TODO: Implement handle_*_internal") indicates that these functions are not yet functional. This severely limits the usability of the codebase, as calls to these functions from JavaScript will silently fail, hindering crucial functionalities.
2. Unhandled .unwrap() and .expect() Calls
Throughout the codebase, .unwrap() and .expect() calls are present. These calls can lead to unexpected program termination (panics) if the underlying conditions are not met. The locations mentioned include merkle.rs, stealth.rs, transaction.rs, and adaptor.rs. These panics can cause node crashes in production environments, making them a significant concern. For instance, merkle.rs:74 can crash if the Merkle tree is empty, and transaction.rs:154, 310 assume a monotonic system clock, which might not always be the case.
3. Debug Prints in VRF Production Code (vrf.rs)
The VRF (Verifiable Random Function) code contains numerous println!() statements, which are debug-level outputs. These statements will flood the logs in a production environment, leading to severe performance degradation and hindering the ability to effectively monitor the system. Furthermore, sensitive cryptographic values may be leaked, increasing the potential for security vulnerabilities.
4. Missing Wallet Persistence Logic (src/lib.rs:428, 442, 464)
The codebase lacks robust wallet persistence logic. This means that wallet states are not being saved or loaded correctly. The TODO comments in src/lib.rs explicitly mention the need to save and load wallet data via JavaScript. Without proper persistence, user keys and wallet data will be lost on restart, rendering the wallet unusable.
5. Incomplete Payment Channel Implementation (payment_channel.rs)
The payment channel implementation is incomplete. The code includes a TODO comment indicating that the full verification of the commitment transaction and adaptor signature is missing. Additionally, the aggregation of blinding factors, crucial for privacy, is also not fully implemented. This incompleteness introduces significant security risks.
🟠 High-Priority Issues
These issues are of high importance and need to be addressed promptly. They can potentially lead to security vulnerabilities, performance bottlenecks, or usability problems.
6. Dangerous Unwrap in Test Code (utils.rs & adaptor.rs)
While .unwrap() calls are common in test code, some are present in library functions that could be called from production code. This creates a risk of unexpected panics in production, especially when the test functions are at the module level, as seen in src/adaptor.rs. Proper isolation of these calls within #[cfg(test)] blocks is critical.
7. Protobuf Domain Mismatch (constants.rs)
A critical mismatch exists in the Protobuf domain definition. DOMAIN_BLOCK_HASH in constants.rs is defined as b"pluribit_block_v2", but src/block.rs:76 uses b"pluribit_block_v3". This discrepancy will cause block hash validation to fail, which can result in chain consensus failure and potentially lead to a fork.
8. Undefined JavaScript/TypeScript Support for BigInt
The codebase's handling of BigInt in JavaScript and TypeScript presents a precision issue. In worker.js, BigInts are converted to Numbers for percentage calculation, which results in a loss of precision for large numbers. The lack of proper handling of BigInt in sync progress reporting can lead to inaccurate results.
9. No Test Suite Configuration
The package.json file indicates a test suite setup, but the actual test.js file is missing. This absence makes it impossible to run the test suite and validate the codebase through continuous integration (CI) processes. Consequently, there's no automated validation.
10. Memory Leak Risk in Caches (src/lib.rs:107-125)
The codebase uses various caches (RECENT_UTXO_CACHE, UTXO_CACHE_LRU, SIDE_BLOCKS, SIDE_BLOCKS_LRU) with manual LRU (Least Recently Used) eviction mechanisms. The WALLET_SESSIONS cache, however, lacks size limits. This unbounded growth can lead to excessive memory consumption, potentially causing the node to crash or become unresponsive when running for an extended time.
11. Timestamp Ordering Vulnerability (src/transaction.rs:148-154)
The transaction validation process relies on the .expect("Time went backwards") call concerning SystemTime::now().duration_since(...). This approach is vulnerable to system clock manipulations. If the system clock goes backward (e.g., due to NTP adjustments or power loss), the node will crash. A clock-safe approach is needed to mitigate this risk.
🟡 Medium-Priority Issues
These issues should be addressed, but they are not as critical as the high-priority ones. They can lead to potential problems in the long run if they are not fixed.
12. Missing Error Handling in Async/Promise Chains (worker.js)
The asynchronous operations in worker.js have limited error handling. Many .catch() handlers merely log errors without providing mechanisms to recover the application state. This can lead to silent failures in crucial operations like block sync and peer connections, which can undermine the node's functionality.
13. Database Schema Evolution Concerns (lib.rs)
Multiple database functions are exposed without any versioning or migration logic. Functions like save_reorg_marker, check_incomplete_reorg, and commit_staged_reorg suggest incomplete reorg handling. This lack of proper schema management poses a risk to hard forks, which can break older database layouts.
14. Incomplete Reorg Recovery Logic (worker.js:840-910)
The reorg recovery logic is extensive, but the code still relies on fallback mechanisms and mentions “manual intervention required” messages. This suggests that the reorg handling is fragile and may not be fully tested under all circumstances. Improving the reliability of reorg handling is crucial for node stability.
15. Dandelion Implementation Gaps (libp2p-node.js)
The Dandelion implementation has privacy-related gaps. When the stem relay is unavailable, the code falls back to direct broadcasting, exposing the sender. Furthermore, failure modes often default to flooding the network, which undermines the privacy guarantees offered by Dandelion.
16. No Rate Limiting on API Endpoints (worker.js:505-773)
There is a lack of rate limiting on API endpoints such as /api/block/:height, /api/tx/:hash, and /api/mempool. While rate limiting might be implemented at the libp2p layer, the code doesn't explicitly configure it. This absence leaves these endpoints vulnerable to denial-of-service (DoS) attacks via API hammering.
17. Incorrect VDF Modulus Fingerprint Check (src/vdf.rs:80)
The VDF modulus fingerprint check is flawed. The condition if !VDF_RSA_MODULUS_SHA256_HEX.is_empty() && calc != VDF_RSA_MODULUS_SHA256_HEX allows bypass if VDF_RSA_MODULUS_SHA256_HEX is an empty string. The check should be revised to ensure proper modulus verification and prevent potential vulnerabilities.
18. Merkle Proof Vulnerability (src/merkle.rs:74-121)
This section uses .unwrap() on tree operations, assuming non-empty inputs. There is no verification that the Merkle proof actually corresponds to a real tree. The lack of validation on Merkle proofs could lead to security risks.
19. Canonical Scalar Enforcement (src/wallet.rs:20-51)
The codebase attempts to enforce canonical form with Scalar::from_canonical_bytes(). However, it's inconsistent across the codebase. src/vrf.rs:117 uses Scalar::from_bytes_mod_order(), which can accept non-canonical scalars. This inconsistency could create different signature verifications, possibly accepting the same proof with different scalars.
🟢 Low-Priority / Improvements
These are minor improvements. Addressing these issues will help increase the quality of the codebase.
20. Debug/Console Logging Left in Production Code
The codebase still includes console.log() and println!() statements in production code. They should be replaced with structured logging or wrapped in #[cfg(debug_assertions)] to prevent unnecessary log pollution in production environments.
21. Block Hash Computation Inconsistency (src/block.rs:96-97)
There's a minor inconsistency in block hash computation. The hash is computed in compute_hash() and immediately recomputed. Changing the structure to use a computed property or validate at struct creation could improve code clarity and efficiency.
22. Missing Input Validation
The code computes the VDF with user-provided input without validating the maximum size. This lack of validation creates a potential risk for the system because malicious input could lead to excessive computation.
23. Incomplete Cut-Through Implementation (src/block.rs:195)
Comments suggest the code will perform cut-through at the block level, but the actual implementation is incomplete. Line 195+ shows aggregation logic that's partially stubbed. A completed cut-through implementation may improve performance.
24. TypeScript Configuration Issues (tsconfig.json)
The TypeScript configuration uses checkJs: true and strict: true, which is good. However, skipLibCheck: true skips validation. Also, the absence of baseUrl or paths configurations hinders import management.
25. Unused Dependencies
The Cargo.toml file includes unused dependencies like base64 = "0.22". Removing these unnecessary dependencies can simplify the build process and decrease the project's size.
26. Hardcoded Addresses/Ports
Bootstrap configuration relies on bootstrap-config.json and bootstrap.testnet.json without providing fallbacks when these files are missing. Also, hardcoded addresses and ports are present. This reliance on hardcoded values can affect the system's flexibility.
27. Work Calculation Truncation Risk (src/blockchain.rs:76-80)
The codebase calculates work using BigUint and then downscales to 64 bits. This downscaling may cause a loss of precision, which can impact the accuracy of the work calculations for high work values.
28. Scalar Serialization Inconsistency (wallet.rs)
The custom scalar_serde module enforces canonical form. However, the custom logic isn't consistently used throughout the code. Different handling of scalar serialization can create inconsistencies, which can impact security.
Summary Table
The following table summarizes the identified issues by severity and primary concern:
| Severity | Count | Primary Concern |
|---|---|---|
| 🔴 Critical | 5 | Unimplemented handlers, panics, domain mismatch, logging spam |
| 🟠 High | 6 | Memory leaks, clock issues, test suite missing, BigInt precision |
| 🟡 Medium | 8 | Error handling, reorg fragility, API DoS, privacy fallbacks |
| 🟢 Low/Improvement | 11 | Code quality, cleanup, minor optimizations |
This audit reveals several critical, high-priority, and medium-priority issues within the Pluribit codebase. Addressing these problems is essential for improving the project's functionality, security, and long-term viability. Furthermore, the findings highlight areas where code quality, performance, and overall maintainability can be enhanced. Prioritizing the resolution of critical and high-priority issues is crucial. Focusing on proper error handling, robust testing, and efficient memory management will contribute to a more stable, secure, and user-friendly product.
For additional resources and more information related to blockchain and security audits, you can visit OpenZeppelin's website.
