diff --git a/miner/core/CMakeLists.txt b/miner/core/CMakeLists.txt index e313dc6..9dcff44 100644 --- a/miner/core/CMakeLists.txt +++ b/miner/core/CMakeLists.txt @@ -118,7 +118,7 @@ set(SOURCES src/net/Network.cpp src/net/strategies/DonateStrategy.cpp src/Summary.cpp - src/xmrig.cpp + src/miner.cpp ) set(SOURCES_CRYPTO diff --git a/miner/core/src/donate.h b/miner/core/src/donate.h index 36b3b77..f543ebe 100644 --- a/miner/core/src/donate.h +++ b/miner/core/src/donate.h @@ -1,6 +1,6 @@ -/* XMRig - * Copyright (c) 2018-2022 SChernykh - * Copyright (c) 2016-2022 XMRig , +/* Miner Platform + * Copyright (c) 2025 Lethean + * Based on XMRig by SChernykh and XMRig team (GPL-3.0-or-later) * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -16,29 +16,18 @@ * along with this program. If not, see . */ -#ifndef XMRIG_DONATE_H -#define XMRIG_DONATE_H +#ifndef MINER_DONATE_H +#define MINER_DONATE_H /* - * Dev donation. + * Dev donation disabled. * - * Percentage of your hashing power that you want to donate to the developer. - * This helps fund ongoing open source development. - * - * Example of how it works for the setting of 1%: - * Your miner will mine into your usual pool for a random time (in a range from 49.5 to 148.5 minutes), - * then switch to the developer's pool for 1 minute, then switch again to your pool for 99 minutes - * and then switch again to developer's pool for 1 minute; these rounds will continue until the miner stops. - * - * Randomised only on the first round to prevent waves on the donation pool. - * - * Switching is instant and only happens after a successful connection, so you never lose any hashes. - * - * XMR: 89qpYgfAZzp8VYKaPbAh1V2vSW9RHCMyHVQxe2oFxZvpK9dF1UMpZSxJK9jikW4QCRGgVni8BJjvTQpJQtHJzYyw8Uz18An + * Miner Platform does not include any built-in developer fees. + * If you'd like to support development, please visit https://lethean.io */ -constexpr const int kDefaultDonateLevel = 1; -constexpr const int kMinimumDonateLevel = 0; // Allow users to opt-out +constexpr const int kDefaultDonateLevel = 0; +constexpr const int kMinimumDonateLevel = 0; -#endif // XMRIG_DONATE_H +#endif // MINER_DONATE_H diff --git a/miner/core/src/xmrig.cpp b/miner/core/src/miner.cpp similarity index 100% rename from miner/core/src/xmrig.cpp rename to miner/core/src/miner.cpp diff --git a/miner/core/src/version.h b/miner/core/src/version.h index 692f693..ad648fc 100644 --- a/miner/core/src/version.h +++ b/miner/core/src/version.h @@ -1,12 +1,12 @@ -/* XMRig - * Copyright (c) 2018-2025 SChernykh - * Copyright (c) 2016-2025 XMRig , +/* Miner Platform + * Copyright (c) 2025 Lethean + * Based on XMRig by SChernykh and XMRig team (GPL-3.0-or-later) * * SPDX-License-Identifier: GPL-3.0-or-later */ -#ifndef XMRIG_VERSION_H -#define XMRIG_VERSION_H +#ifndef MINER_VERSION_H +#define MINER_VERSION_H #define APP_ID "miner" #define APP_NAME "Miner" @@ -84,4 +84,4 @@ # define APP_BITS "32 bit" #endif -#endif // XMRIG_VERSION_H +#endif // MINER_VERSION_H diff --git a/miner/cuda/CLAUDE.md b/miner/cuda/CLAUDE.md index 5e4f8ed..c2595ea 100644 --- a/miner/cuda/CLAUDE.md +++ b/miner/cuda/CLAUDE.md @@ -4,7 +4,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co ## Project Overview -miner-cuda (xmrig-cuda fork) is a CUDA plugin for XMRig cryptocurrency miner, providing NVIDIA GPU acceleration. It compiles to a shared library (`libminer-cuda.so` on Linux, `miner-cuda.dll` on Windows) that XMRig loads at runtime. +miner-cuda is a CUDA plugin for Miner Platform, providing NVIDIA GPU acceleration. It compiles to a shared library (`libminer-cuda.so` on Linux, `miner-cuda.dll` on Windows) that the miner loads at runtime. ## Build Commands @@ -39,9 +39,9 @@ cmake -DCUDA_COMPILER=clang .. ## Architecture -### Plugin Interface (`src/xmrig-cuda.h`) +### Plugin Interface (`src/miner-cuda.h`) -C-linkage API that XMRig calls. Key functions: +C-linkage API that the miner calls. Key functions: - `alloc()`/`release()` - GPU context lifecycle - `deviceInfo()`/`deviceInit()` - GPU detection and initialization - `setJob()` - Configure algorithm and job data @@ -86,15 +86,15 @@ Automatically configured based on CUDA toolkit version: | `WITH_CN_FEMTO` | ON | CryptoNight-UPX2 algorithm | | `WITH_ARGON2` | OFF | Argon2 family (unsupported) | | `CUDA_ARCH` | auto | GPU compute capabilities to target | -| `XMRIG_LARGEGRID` | ON | Support >128 CUDA blocks | +| `MINER_LARGEGRID` | ON | Support >128 CUDA blocks | | `CUDA_COMPILER` | nvcc | CUDA compiler (nvcc or clang) | ## File Layout ``` src/ -├── xmrig-cuda.cpp # Main plugin implementation -├── xmrig-cuda.h # C API header (exported functions) +├── miner-cuda.cpp # Main plugin implementation +├── miner-cuda.h # C API header (exported functions) ├── cryptonight.h # nvid_ctx struct and GPU context ├── cuda_core.cu # Core CUDA kernels ├── cuda_extra.cu # CUDA utilities and memory management diff --git a/miner/cuda/CMakeLists.txt b/miner/cuda/CMakeLists.txt index 31f3271..945ba38 100644 --- a/miner/cuda/CMakeLists.txt +++ b/miner/cuda/CMakeLists.txt @@ -80,8 +80,8 @@ set(SOURCES src/crypto/common/Algorithm.cpp src/crypto/common/Algorithm.h src/version.h - src/xmrig-cuda.cpp - src/xmrig-cuda.h + src/miner-cuda.cpp + src/miner-cuda.h ) diff --git a/miner/cuda/TODO.md b/miner/cuda/TODO.md new file mode 100644 index 0000000..4ef05d1 --- /dev/null +++ b/miner/cuda/TODO.md @@ -0,0 +1,512 @@ +# Code Review Findings - CUDA Mining Plugin Enterprise Audit + +**Generated:** 2025-12-31 +**Reviewed by:** 6 Parallel Opus Code Reviewers +**Target:** XMRig-CUDA Plugin (76 source files) + +--- + +## Summary + +| Domain | Critical | High | Medium | Total | +|--------|----------|------|--------|-------| +| Plugin API & Entry Point | 2 | 4 | 2 | 8 | +| CUDA Core Kernels | 2 | 3 | 2 | 7 | +| CryptoNight-R NVRTC | 1 | 3 | 3 | 7 | +| RandomX CUDA | 2 | 3 | 1 | 6 | +| KawPow CUDA | 2 | 5 | 1 | 8 | +| Crypto Common | 3 | 3 | 0 | 6 | +| **TOTAL** | **12** | **21** | **9** | **42** | + +--- + +## Critical Issues + +### CRIT-001: Memory Leak in DatasetHost::release() +- **File:** `src/xmrig-cuda.cpp:62-72` +- **Domain:** Plugin API & Entry Point +- **Confidence:** 95% + +The `m_ptr` is set to `nullptr` unconditionally after decrementing `m_refs`, even when `m_refs > 0`. This loses the pointer while other contexts may still be using it. + +```cpp +inline void release() +{ + --m_refs; + if (m_refs == 0) { + cudaHostUnregister(m_ptr); + } + m_ptr = nullptr; // BUG: Always sets to nullptr, even when refs > 0 +} +``` + +**Fix:** Only set `m_ptr = nullptr` inside the `if (m_refs == 0)` block. + +--- + +### CRIT-002: Unchecked cudaHostUnregister Error +- **File:** `src/xmrig-cuda.cpp:68-69` +- **Domain:** Plugin API & Entry Point +- **Confidence:** 90% + +`cudaHostUnregister()` is called without error checking, unlike all other CUDA API calls in the codebase which use `CUDA_CHECK` macro. + +**Fix:** Add error checking or at minimum log errors from cleanup path. + +--- + +### CRIT-003: Race Condition with atomicExch in MUL_SUM_XOR_DST +- **File:** `src/cuda_extra.h:98` +- **Domain:** CUDA Core Kernels +- **Confidence:** 85% + +The macro reads `dst0` non-atomically at the start, but another thread may be writing to the same location via atomicExch. This creates a read-write race. + +**Fix:** Add `__threadfence()` and ensure all accesses to shared `dst` are atomic. + +--- + +### CRIT-004: Shared Memory Array Index Bounds Overflow +- **File:** `src/cuda_core.cu:335` +- **Domain:** CUDA Core Kernels +- **Confidence:** 82% + +Expression `(MASK - 0x30)` could underflow if MASK < 0x30 for certain algorithm variants, leading to out-of-bounds scratchpad access. + +**Fix:** Add bounds validation: `const uint32_t safe_mask = (MASK >= 0x30) ? (MASK - 0x30) : 0;` + +--- + +### CRIT-005: Memory Leak in Module Cleanup Path +- **File:** `src/cuda_core.cu:834-836` +- **Domain:** CryptoNight-R NVRTC +- **Confidence:** 95% + +When changing block height, the old module is unloaded before checking if compilation succeeds. If `CryptonightR_get_program()` throws, `ctx->module` is left in invalid state. + +**Fix:** Get new module first, then unload old module only after successful compilation. + +--- + +### CRIT-006: Dataset Buffer Overflow in execute_vm Kernel +- **File:** `src/RandomX/randomx_cuda.hpp:2175` +- **Domain:** RandomX CUDA +- **Confidence:** 95% + +Dataset access at `ma + sub * 8` can reach maximum offset of 2,181,038,008 bytes with only 8 bytes safety margin. Under-allocation by even a few bytes causes out-of-bounds read. + +**Fix:** Add explicit bounds checking before dataset access. + +--- + +### CRIT-007: Scratchpad Buffer Overflow Risk +- **File:** `src/RandomX/randomx_cuda.hpp:2132-2133` +- **Domain:** RandomX CUDA +- **Confidence:** 90% + +Scratchpad pointer arithmetic `spAddr + sub * 8` relies on exact allocation sizes. If `RANDOMX_SCRATCHPAD_L3` constant is misconfigured in any coin variant, buffer overflow occurs. + +**Fix:** Add compile-time static_assert and runtime bounds checks. + +--- + +### CRIT-008: Out-of-Bounds Array Access in dag_sizes +- **File:** `src/KawPow/raven/CudaKawPow_gen.cpp:432` +- **Domain:** KawPow CUDA +- **Confidence:** 95% + +`dag_sizes[epoch]` accessed without validating that `epoch` is within array bounds. The `dag_sizes` parameter is a raw pointer with no size information. + +**Fix:** Add size parameter to function signature and validate bounds before access. + +--- + +### CRIT-009: Background Thread Never Joins/Terminates +- **File:** `src/KawPow/raven/CudaKawPow_gen.cpp:51-82` +- **Domain:** KawPow CUDA +- **Confidence:** 100% + +Background thread runs infinite `for(;;)` loop with no exit condition. Thread is never joined or deleted on shutdown. + +**Fix:** Add `std::atomic shutdown` flag, check in loop, and add cleanup function. + +--- + +### CRIT-010: Integer Overflow in Algorithm L3 Memory Calculation +- **File:** `src/crypto/common/Algorithm.h:89` +- **Domain:** Crypto Common +- **Confidence:** 85% + +The `l3()` function uses `1ULL << ((id >> 16) & 0xff)`. If bits 16-23 are >= 64, this causes undefined behavior due to shift overflow. + +**Fix:** Add bounds checking: `return (shift < 64) ? (1ULL << shift) : 0;` + +--- + +### CRIT-011: Division by Zero in VARIANT2_INTEGER_MATH +- **File:** `src/crypto/cn/CryptoNight_monero.h:79,81` +- **Domain:** Crypto Common +- **Confidence:** 90% + +While divisor `d` is OR'd with `0x80000001UL`, the ARM version (line 125-127) has different logic that may not guarantee non-zero divisor. + +**Fix:** Add explicit validation `if (d == 0) { /* handle error */ }`. + +--- + +### CRIT-012: Missing Function Definition for int_sqrt_v2 +- **File:** `src/crypto/cn/CryptoNight_monero.h:83` +- **Domain:** Crypto Common +- **Confidence:** 95% + +CUDA plugin calls `int_sqrt_v2()` but function is not defined in CUDA codebase. It exists in core project at separate compilation unit. + +**Fix:** Include proper header or provide implementation in CUDA plugin headers. + +--- + +## High Priority Issues + +### HIGH-001: Integer Overflow in Scratchpad Size Calculation +- **File:** `src/RandomX/randomx.cu:33` +- **Domain:** Plugin API & Entry Point +- **Confidence:** 85% + +`batch_size * (ctx->algorithm.l3() + 64)` - both operands may be uint32_t, causing overflow before assignment to uint64_t. + +**Fix:** Cast to uint64_t first: `static_cast(batch_size) * ...` + +--- + +### HIGH-002: Incomplete Error Cleanup in cryptonight_extra_cpu_init() +- **File:** `src/cuda_extra.cu:313-389` +- **Domain:** Plugin API & Entry Point +- **Confidence:** 90% + +Multiple GPU allocations with CUDA_CHECK (which throws on error) but no try-catch cleanup. Mid-function failures leak prior allocations. + +**Fix:** Add RAII wrapper or manual cleanup in catch block. + +--- + +### HIGH-003: Missing Null Check in deviceName() +- **File:** `src/xmrig-cuda.cpp:369-372` +- **Domain:** Plugin API & Entry Point +- **Confidence:** 85% + +`deviceName()` dereferences `ctx` without null check, while `deviceInt()` does check. Inconsistent defensive programming. + +**Fix:** Add `if (ctx == nullptr) { return nullptr; }`. + +--- + +### HIGH-004: Potential Double-Free in release() with d_ctx_state2 +- **File:** `src/xmrig-cuda.cpp:537-538` +- **Domain:** Plugin API & Entry Point +- **Confidence:** 80% + +In non-HEAVY algorithms, `d_ctx_state2` aliases `d_ctx_state`. But `release()` unconditionally calls `cudaFree()` on both. + +**Fix:** Check if pointers are equal before freeing second. + +--- + +### HIGH-005: Missing CUDA Error Check After Kernel Launches +- **File:** `src/cuda_core.cu:741-805` +- **Domain:** CUDA Core Kernels +- **Confidence:** 95% + +Dynamic shared memory size calculation could exceed device limits (48KB), but no runtime validation before launch. + +**Fix:** Add `cudaDeviceGetAttribute` check for max shared memory before launch. + +--- + +### HIGH-006: Integer Overflow in Index Calculation (CN_HEAVY) +- **File:** `src/cuda_core.cu:621-622` +- **Domain:** CUDA Core Kernels +- **Confidence:** 90% + +Expression `((idx0 & MASK) >> 3) + 1u` can overflow for large idx0, exceeding allocated `long_state` buffer size. + +**Fix:** Add overflow check before calculation. + +--- + +### HIGH-007: Uncoalesced Memory Access Pattern in Phase 2 +- **File:** `src/cuda_core.cu:337-372` +- **Domain:** CUDA Core Kernels +- **Confidence:** 88% + +Adjacent threads access non-adjacent memory locations, causing ~50% reduction in memory bandwidth utilization. + +**Fix:** Restructure to use shared memory tiling with coalesced loads. + +--- + +### HIGH-008: Unvalidated Height Parameter in Code Generation +- **File:** `src/CudaCryptonightR_gen.cpp:261-262` +- **Domain:** CryptoNight-R NVRTC +- **Confidence:** 85% + +The `height` parameter seeds Blake hash directly without validation. Extreme height values near UINT64_MAX are not rejected. + +**Fix:** Add validation for reasonable blockchain height limits. + +--- + +### HIGH-009: Missing PTX Data Validation Before Module Load +- **File:** `src/cuda_core.cu:840-843` +- **Domain:** CryptoNight-R NVRTC +- **Confidence:** 90% + +If `CryptonightR_get_program()` returns early, `ptx` vector may be empty. Loading empty PTX causes undefined behavior. + +**Fix:** Check `if (ptx.empty() || lowered_name.empty())` before `cuModuleLoadDataEx`. + +--- + +### HIGH-010: Missing Memory Deallocation for RandomX Buffers +- **File:** `src/RandomX/randomx.cu:30-48` +- **Domain:** RandomX CUDA +- **Confidence:** 100% + +`randomx_prepare` allocates 5 device memory buffers but no cleanup function exists for re-calling with different batch sizes. + +**Fix:** Add `randomx_cleanup()` function and proper lifecycle management. + +--- + +### HIGH-011: Uninitialized VM State Memory +- **File:** `src/RandomX/randomx_cuda.hpp:448-449` +- **Domain:** RandomX CUDA +- **Confidence:** 85% + +`init_vm` kernel only zeros R[0-7], but imm_buf and compiled_program sections left with uninitialized data. + +**Fix:** Add `memset(R, 0, VM_STATE_SIZE)` for full initialization. + +--- + +### HIGH-012: Integer Overflow in Scratchpad Size Calculation +- **File:** `src/RandomX/randomx.cu:33` +- **Domain:** RandomX CUDA +- **Confidence:** 80% + +Same as HIGH-001: `batch_size * (ctx->algorithm.l3() + 64)` could overflow if operands are uint32_t. + +**Fix:** Use `static_cast(batch_size) * ...`. + +--- + +### HIGH-013: Missing Null Pointer Validation for dag_sizes +- **File:** `src/KawPow/raven/CudaKawPow_gen.cpp:405-432` +- **Domain:** KawPow CUDA +- **Confidence:** 90% + +`dag_sizes` pointer is dereferenced without null checking. Background lambda captures pointer by value - could be null. + +**Fix:** Add `if (!dag_sizes) { CUDA_THROW("dag_sizes cannot be null"); }`. + +--- + +### HIGH-014: Integer Overflow in DAG Element Calculation +- **File:** `src/KawPow/raven/CudaKawPow_gen.cpp:432` +- **Domain:** KawPow CUDA +- **Confidence:** 85% + +`dag_elements` is uint64_t but implicitly converted to uint32_t in `calculate_fast_mod_data()`. Values exceeding 32-bit range silently truncate. + +**Fix:** Validate `dag_elements <= UINT32_MAX` before conversion. + +--- + +### HIGH-015: Unchecked string::find in Code Generation +- **File:** `src/KawPow/raven/CudaKawPow_gen.cpp:423,426,448,455` +- **Domain:** KawPow CUDA +- **Confidence:** 90% + +All four `source_code.replace()` calls use `find()` without checking for `std::string::npos`. Missing template markers cause undefined behavior. + +**Fix:** Create helper function that validates find() result before replace(). + +--- + +### HIGH-016: Module Unload Without Null Check +- **File:** `src/KawPow/raven/KawPow.cu:99-101` +- **Domain:** KawPow CUDA +- **Confidence:** 80% + +Module is unloaded but NOT set to nullptr afterwards. Creates use-after-free if period changes multiple times. + +**Fix:** Set `ctx->kawpow_module = nullptr` after `cuModuleUnload()`. + +--- + +### HIGH-017: Missing Error Check on Large DAG Allocation +- **File:** `src/KawPow/raven/KawPow.cu:58-62` +- **Domain:** KawPow CUDA +- **Confidence:** 85% + +DAGs can be multi-GB. No pre-allocation validation that GPU has sufficient memory. + +**Fix:** Use `cudaMemGetInfo()` to verify available memory before allocation. + +--- + +### HIGH-018: Unvalidated Array Index in v4_random_math Macro +- **File:** `src/crypto/cn/r/variant4_random_math.h:104-106` +- **Domain:** Crypto Common +- **Confidence:** 85% + +`r[op->src_index]` and `r[op->dst_index]` accessed without bounds checking against 9-element array. + +**Fix:** Add `if (op->src_index >= 9 || op->dst_index >= 9) { return; }`. + +--- + +### HIGH-019: Integer Overflow in L3 Mask Calculation +- **File:** `src/crypto/cn/CnAlgo.h:111` +- **Domain:** Crypto Common +- **Confidence:** 82% + +If `Algorithm::l3(algo)` returns 0, then `l3(algo) - 1` underflows to SIZE_MAX. + +**Fix:** Add validation: `if (l3_val == 0) return 0;`. + +--- + +### HIGH-020: Potential Buffer Overflow in blake256_update +- **File:** `src/crypto/cn/c_blake256.c:150` +- **Domain:** Crypto Common +- **Confidence:** 80% + +`memcpy((void *) (S->buf + left), ...)` - if `left + (datalen >> 3)` exceeds 64 (buf size), overflow occurs. + +**Fix:** Add explicit bounds check before memcpy. + +--- + +## Medium Priority Issues + +### MED-001: Missing Error Check on strdup() +- **File:** `src/cuda_extra.cu:550` +- **Domain:** Plugin API & Entry Point +- **Confidence:** 85% + +`strdup()` can return nullptr on allocation failure but this is not checked. + +--- + +### MED-002: Unchecked CUDA Error in init() +- **File:** `src/xmrig-cuda.cpp:513-518` +- **Domain:** Plugin API & Entry Point +- **Confidence:** 80% + +`cuInit(0)` called without checking return value. + +--- + +### MED-003: Missing Synchronization After Shuffle Operations +- **File:** `src/cuda_core.cu:354-357` +- **Domain:** CUDA Core Kernels +- **Confidence:** 85% + +`__syncwarp()` for CUDA 9+ only synchronizes within warp. If `block2.x > 32`, shared memory access across warps has race condition. + +--- + +### MED-004: Device Memory Leak on Algorithm Change +- **File:** `src/cuda_core.cu:834-836` +- **Domain:** CUDA Core Kernels +- **Confidence:** 92% + +Module unloaded for CN-R variant switching but associated device memory allocations not cleaned up. + +--- + +### MED-005: Race Condition in Cache Access (TOCTOU) +- **File:** `src/CudaCryptonightR_gen.cpp:158-171,268-280` +- **Domain:** CryptoNight-R NVRTC +- **Confidence:** 85% + +Check-then-act pattern allows duplicate NVRTC compilations under high concurrency. + +--- + +### MED-006: Background Thread Never Joins (CN-R) +- **File:** `src/CudaCryptonightR_gen.cpp:96,124-126` +- **Domain:** CryptoNight-R NVRTC +- **Confidence:** 100% + +Same issue as CRIT-009 but in CryptoNight-R code path. + +--- + +### MED-007: Insufficient Error Context in NVRTC Failures +- **File:** `src/CudaCryptonightR_gen.cpp:190-205` +- **Domain:** CryptoNight-R NVRTC +- **Confidence:** 90% + +Error log missing height/arch context, making production debugging difficult. + +--- + +### MED-008: Race Condition in find_shares Kernel +- **File:** `src/RandomX/hash.hpp:22-32` +- **Domain:** RandomX CUDA +- **Confidence:** 82% + +Multiple threads using `atomicInc` could theoretically get same idx before increment completes on all SMs. + +--- + +### MED-009: Race Condition in Background Compilation +- **File:** `src/KawPow/raven/CudaKawPow_gen.cpp:407-410` +- **Domain:** KawPow CUDA +- **Confidence:** 80% + +Lambda captures `dag_sizes` pointer by value. Use-after-free if caller deallocates before background execution. + +--- + +## Recommended Priority Order + +### Immediate (Security Critical) +1. CRIT-006: Dataset buffer overflow in RandomX +2. CRIT-007: Scratchpad buffer overflow in RandomX +3. CRIT-008: Out-of-bounds dag_sizes access in KawPow +4. CRIT-012: Missing int_sqrt_v2 function +5. CRIT-011: Division by zero in VARIANT2 + +### This Week (Data Integrity) +6. CRIT-001: DatasetHost memory leak +7. CRIT-003: Race condition in MUL_SUM_XOR_DST +8. CRIT-009: Background thread never terminates +9. HIGH-010: RandomX buffer cleanup +10. HIGH-002: cryptonight_extra_cpu_init cleanup + +### Next Sprint (Stability) +11. CRIT-005: Module cleanup path memory leak +12. CRIT-010: Integer overflow in l3() calculation +13. HIGH-005: Missing kernel launch error checks +14. HIGH-015: Unchecked string::find in code gen +15. HIGH-016: Module use-after-free + +### Backlog (Quality) +- All Medium priority items +- Performance optimization (HIGH-007) +- Error message improvements (MED-007) + +--- + +## Review Completion Status + +- [x] Plugin API & Entry Point - 8 issues found +- [x] CUDA Core Kernels - 7 issues found +- [x] CryptoNight-R NVRTC - 7 issues found +- [x] RandomX CUDA - 6 issues found +- [x] KawPow CUDA - 8 issues found +- [x] Crypto Common - 6 issues found + +**Total Issues Identified: 42** diff --git a/miner/cuda/src/xmrig-cuda.cpp b/miner/cuda/src/miner-cuda.cpp similarity index 99% rename from miner/cuda/src/xmrig-cuda.cpp rename to miner/cuda/src/miner-cuda.cpp index 7d8a6b5..563d9dc 100644 --- a/miner/cuda/src/xmrig-cuda.cpp +++ b/miner/cuda/src/miner-cuda.cpp @@ -26,7 +26,7 @@ #include "cryptonight.h" #include "cuda_device.hpp" #include "version.h" -#include "xmrig-cuda.h" +#include "miner-cuda.h" #include diff --git a/miner/cuda/src/xmrig-cuda.h b/miner/cuda/src/miner-cuda.h similarity index 100% rename from miner/cuda/src/xmrig-cuda.h rename to miner/cuda/src/miner-cuda.h diff --git a/miner/cuda/src/version.h b/miner/cuda/src/version.h index d35f34e..46f207c 100644 --- a/miner/cuda/src/version.h +++ b/miner/cuda/src/version.h @@ -1,6 +1,6 @@ -/* XMRig - * Copyright (c) 2018-2025 SChernykh - * Copyright (c) 2016-2025 XMRig , +/* Miner Platform - CUDA Plugin + * Copyright (c) 2025 Lethean + * Based on XMRig-CUDA by SChernykh and XMRig team (GPL-3.0-or-later) * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -16,8 +16,8 @@ * along with this program. If not, see . */ -#ifndef XMRIG_VERSION_H -#define XMRIG_VERSION_H +#ifndef MINER_VERSION_H +#define MINER_VERSION_H #define APP_ID "miner-cuda" #define APP_NAME "Miner" @@ -33,4 +33,4 @@ #define API_VERSION 4 -#endif /* XMRIG_VERSION_H */ +#endif /* MINER_VERSION_H */ diff --git a/miner/proxy/CLAUDE.md b/miner/proxy/CLAUDE.md index 6805bf4..0c67a0d 100644 --- a/miner/proxy/CLAUDE.md +++ b/miner/proxy/CLAUDE.md @@ -35,7 +35,7 @@ rm -rf build && mkdir build && cd build && cmake .. && make -j$(nproc) ## Architecture Overview -XMRig Proxy is a high-performance CryptoNote stratum protocol proxy that can handle 100K+ miner connections while maintaining minimal pool-side connections through nonce splitting. +Miner Proxy is a high-performance CryptoNote stratum protocol proxy that can handle 100K+ miner connections while maintaining minimal pool-side connections through nonce splitting. ### Data Flow @@ -112,13 +112,13 @@ Extensions in `doc/STRATUM_EXT.md`: algorithm negotiation, rig identifiers, Nice ### Key Defines ```cpp -XMRIG_PROXY_PROJECT // Proxy-specific code paths -XMRIG_FORCE_TLS // TLS enforcement +MINER_PROXY_PROJECT // Proxy-specific code paths +MINER_FORCE_TLS // TLS enforcement APP_DEVEL // Development features (enables printState()) APP_DEBUG // Debug logging (set via WITH_DEBUG_LOG) -XMRIG_ALGO_RANDOMX // Algorithm support flags -XMRIG_FEATURE_HTTP // HTTP API enabled -XMRIG_FEATURE_API // REST API enabled +MINER_ALGO_RANDOMX // Algorithm support flags +MINER_FEATURE_HTTP // HTTP API enabled +MINER_FEATURE_API // REST API enabled ``` ## Configuration diff --git a/miner/proxy/CMakeLists.txt b/miner/proxy/CMakeLists.txt index ac8c2dc..d9c8202 100644 --- a/miner/proxy/CMakeLists.txt +++ b/miner/proxy/CMakeLists.txt @@ -110,7 +110,7 @@ set(SOURCES src/proxy/workers/Worker.cpp src/proxy/workers/Workers.cpp src/Summary.cpp - src/xmrig.cpp + src/miner.cpp ) if (WIN32) diff --git a/miner/proxy/TODO.md b/miner/proxy/TODO.md index 0cd844e..067f88a 100644 --- a/miner/proxy/TODO.md +++ b/miner/proxy/TODO.md @@ -2,80 +2,626 @@ **Generated:** 2025-12-31 **Reviewed by:** 8 Parallel Opus Code Reviewers -**Target:** XMRig-based C++ Stratum Proxy +**Target:** XMRig-based C++ Stratum Proxy (347 source files) --- -## Review Domains - -- [ ] Entry Point & App Lifecycle -- [ ] Core Controller & Config -- [ ] Proxy Core (Server, Miner, Login, Stats) -- [ ] Proxy TLS & Workers -- [ ] Splitter System (NiceHash, Simple, ExtraNonce, Donate) -- [ ] Network & Stratum Client -- [ ] HTTP/HTTPS & REST API -- [ ] Base I/O & Kernel Infrastructure - ## Summary | Domain | Critical | High | Medium | Total | |--------|----------|------|--------|-------| -| Entry Point & App Lifecycle | - | - | - | - | -| Core Controller & Config | - | - | - | - | -| Proxy Core | - | - | - | - | -| Proxy TLS & Workers | - | - | - | - | -| Splitter System | - | - | - | - | -| Network & Stratum Client | - | - | - | - | -| HTTP/HTTPS & REST API | - | - | - | - | -| Base I/O & Kernel | - | - | - | - | -| **TOTAL** | **-** | **-** | **-** | **-** | +| Entry Point & App Lifecycle | 2 | 2 | 2 | 6 | +| Core Controller & Config | 1 | 4 | 1 | 6 | +| Proxy Core (Server, Miner, Events) | 4 | 5 | 1 | 10 | +| Proxy TLS & Workers | 3 | 2 | 2 | 7 | +| Splitter System | 2 | 3 | 0 | 5 | +| Network & Stratum Client | 3 | 5 | 1 | 9 | +| HTTP/HTTPS & REST API | 1 | 3 | 3 | 7 | +| Base I/O & Kernel | 2 | 2 | 3 | 7 | +| **TOTAL** | **18** | **26** | **13** | **57** | --- ## Critical Issues -_Pending review..._ +### CRIT-001: Double-Delete in Controller Destructor and stop() +- **File:** `src/core/Controller.cpp:45,73-74` +- **Domain:** Entry Point & App Lifecycle +- **Confidence:** 100% + +`m_proxy` deleted in both destructor and `stop()` method. If `stop()` called before destructor, double-free causes crash/heap corruption. + +**Fix:** Add null check in destructor or stop(), set to nullptr after delete. + +--- + +### CRIT-002: UV Event Loop Closed Without Draining Handles +- **File:** `src/App.cpp:78-79` +- **Domain:** Entry Point & App Lifecycle +- **Confidence:** 95% + +`uv_loop_close()` called immediately after `uv_run()` without ensuring handles closed. Returns UV_EBUSY (ignored), leaking resources. + +**Fix:** Loop until `uv_loop_close()` succeeds, calling `uv_run(UV_RUN_ONCE)`. + +--- + +### CRIT-003: Missing JSON Type Validation in BindHost Constructor +- **File:** `src/proxy/BindHost.cpp:67-72` +- **Domain:** Core Controller & Config +- **Confidence:** 95% + +Direct `GetString()`, `GetUint()`, `GetBool()` calls without checking field existence/type. Crashes on malformed config (DoS). + +**Fix:** Add `HasMember()` and type checks before accessing JSON fields. + +--- + +### CRIT-004: Race Condition in Events System - Non-Atomic Ready Flag +- **File:** `src/proxy/Events.cpp:37-56` +- **Domain:** Proxy Core +- **Confidence:** 95% + +`m_ready` flag is bool, not atomic. Multiple threads can pass check simultaneously, causing event corruption. + +**Fix:** Use `std::atomic` or mutex to protect event dispatch. + +--- + +### CRIT-005: Memory Pool (MemPool) Not Thread-Safe +- **File:** `src/base/net/tools/MemPool.h:45-73` +- **Domain:** Proxy Core +- **Confidence:** 100% + +`allocate()` and `deallocate()` modify shared STL containers without synchronization. Called from libuv callbacks (multi-threaded). Heap corruption guaranteed under load. + +**Fix:** Add mutex to protect all MemPool operations. + +--- + +### CRIT-006: Static Event Buffer Shared Across All Events +- **File:** `src/proxy/events/Event.h:52` +- **Domain:** Proxy Core +- **Confidence:** 90% + +All events use single static 4KB buffer with placement new. Concurrent events corrupt each other's memory. + +**Fix:** Use heap allocation for events or implement thread-safe event pool. + +--- + +### CRIT-007: Storage Counter Overflow - ID Collision +- **File:** `src/base/net/tools/Storage.h:37-42,81` +- **Domain:** Proxy Core +- **Confidence:** 85% + +`m_counter` increments without bounds check. After 2^32/2^64 connections, IDs wrap causing wrong miner deletion, use-after-free. + +**Fix:** Add overflow detection and ID recycling mechanism. + +--- + +### CRIT-008: Unchecked SSL_write Return Value +- **File:** `src/base/net/tls/ServerTls.cpp:65` +- **Domain:** TLS & Workers +- **Confidence:** 90% + +`SSL_write()` return value ignored. Silent data loss, corrupted protocol messages. + +**Fix:** Check return value, handle partial writes and errors. + +--- + +### CRIT-009: TLS setCiphers() Returns True on Failure +- **File:** `src/base/net/tls/TlsContext.cpp:165-174` +- **Domain:** TLS & Workers +- **Confidence:** 100% + +Copy-paste bug: function logs error but returns `true` on cipher config failure. Server runs with weak default ciphers. + +**Fix:** Return `false` on line 173. + +--- + +### CRIT-010: Unbounded Memory Growth in m_results Map +- **File:** `src/proxy/splitters/nicehash/NonceMapper.cpp:148,264-276` +- **Domain:** Splitter System +- **Confidence:** 95% + +Submit contexts stored in map, only removed on pool response. Network issues = unbounded memory growth. + +**Fix:** Add timestamp to SubmitCtx, cleanup stale entries in gc(). + +--- + +### CRIT-011: NonceSplitter gc() Vector Out-of-Bounds Access +- **File:** `src/proxy/splitters/nicehash/NonceSplitter.cpp:93-97` +- **Domain:** Splitter System +- **Confidence:** 90% + +While loop calls `m_upstreams.back()` without empty check. If all mappers suspended, crashes on empty vector. + +**Fix:** Add `!m_upstreams.empty()` to while condition. + +--- + +### CRIT-012: Unchecked SSL_write/BIO_write in Stratum TLS +- **File:** `src/base/net/stratum/Tls.cpp:84-89,104-107` +- **Domain:** Network & Stratum +- **Confidence:** 95% + +Return values ignored. Silent data loss, TLS state corruption. + +**Fix:** Check return values, handle errors appropriately. + +--- + +### CRIT-013: Missing TLS Certificate Verification +- **File:** `src/base/net/stratum/Tls.cpp:35-48` +- **Domain:** Network & Stratum +- **Confidence:** 100% + +No `SSL_CTX_set_verify()` call. Certificates not validated unless fingerprint provided. Vulnerable to MITM attacks. + +**Fix:** Add `SSL_CTX_set_verify(m_ctx, SSL_VERIFY_PEER, nullptr)`. + +--- + +### CRIT-014: Timing Attack in API Token Authentication +- **File:** `src/base/api/Httpd.cpp:193-197` +- **Domain:** HTTP API +- **Confidence:** 100% + +Uses `strncmp()` for token comparison. Attacker can extract token character-by-character via timing. + +**Fix:** Use `CRYPTO_memcmp()` for constant-time comparison. + +--- + +### CRIT-015: Race Condition in Signal Handler +- **File:** `src/base/io/Signals.cpp:61-88` +- **Domain:** Base I/O & Kernel +- **Confidence:** 95% + +Signal handler calls `LOG_WARN()` which takes mutex, allocates memory. Not async-signal-safe. Deadlock or heap corruption. + +**Fix:** Only forward signal to listener, log in main event loop context. + +--- + +### CRIT-016: Potential Buffer Overflow in Log Formatting +- **File:** `src/base/io/log/Log.cpp:96-101` +- **Domain:** Base I/O & Kernel +- **Confidence:** 85% + +Magic number `32` in buffer size calculation. Large timestamps + messages can underflow available size. + +**Fix:** Add explicit bounds checking before vsnprintf. + +--- + +### CRIT-017: Private Key File Written with Insecure Permissions +- **File:** `src/base/net/tls/TlsGen.cpp:128-134` +- **Domain:** TLS & Workers +- **Confidence:** 90% + +Private key file created with default permissions (0644 = world-readable). + +**Fix:** Add `chmod(m_certKey, 0600)` on Unix. + +--- + +### CRIT-018: Missing NULL Check in BindHost JSON Constructor (Duplicate) +- **File:** `src/proxy/BindHost.cpp:67,71-72` +- **Domain:** TLS & Workers +- **Confidence:** 95% + +Same as CRIT-003 - found by multiple reviewers, confirming severity. --- ## High Priority Issues -_Pending review..._ +### HIGH-001: Missing uv_stop() in Shutdown Path +- **File:** `src/App.cpp:121-129` +- **Domain:** Entry Point +- **Confidence:** 85% + +`close()` doesn't call `uv_stop()`. UV loop continues until handles naturally close. Delayed/hung shutdown. + +--- + +### HIGH-002: Use-After-Free Risk in Signal/Console Callbacks +- **File:** `src/base/io/Signals.cpp:87`, `Console.cpp:74` +- **Domain:** Entry Point +- **Confidence:** 80% + +`m_listener` accessed after `App::close()` resets handles. Race between close and pending events. + +--- + +### HIGH-003: Integer Overflow in strtol Conversion +- **File:** `src/core/config/ConfigTransform.cpp:85` +- **Domain:** Config +- **Confidence:** 85% + +`strtol()` cast to `uint64_t` without overflow/error checking. Negative values wrap, no error detection. + +--- + +### HIGH-004: Port Number Parsing Without Bounds Check +- **File:** `src/proxy/BindHost.cpp:136,158` +- **Domain:** Config +- **Confidence:** 90% + +Port parsed via `strtol()`, cast to `uint16_t` without validating 0-65535 range. + +--- + +### HIGH-005: Double-Delete Risk in Controller (Config Review) +- **File:** `src/core/Controller.cpp:43-46,69-75` +- **Domain:** Config +- **Confidence:** 85% + +Same as CRIT-001, confirmed by second reviewer. + +--- + +### HIGH-006: Missing Null Pointer Check in Controller Methods +- **File:** `src/core/Controller.cpp:65,78-99` +- **Domain:** Config +- **Confidence:** 90% + +`proxy()` returns potentially null `m_proxy` without checks. Crashes if called before `init()`. + +--- + +### HIGH-007: Unbounded Vector Growth in StatsData::latency +- **File:** `src/proxy/StatsData.h:96,138` +- **Domain:** Proxy Core +- **Confidence:** 100% + +One entry per accepted share, forever. Memory exhaustion guaranteed. + +--- + +### HIGH-008: NULL Dereference in Server::create() - Dead Code +- **File:** `src/proxy/Server.cpp:89-92` +- **Domain:** Proxy Core +- **Confidence:** 80% + +`new` throws on failure, doesn't return NULL. Check is dead code, real failures unhandled. + +--- + +### HIGH-009: Missing Validation in Miner::parseRequest() +- **File:** `src/proxy/Miner.cpp:354-355` +- **Domain:** Proxy Core +- **Confidence:** 85% + +`doc["method"].GetString()` called without validating field exists. Crash on malformed client request. + +--- + +### HIGH-010: Non-Atomic Counters in Counters Class +- **File:** `src/proxy/Counters.h:42-67` +- **Domain:** Proxy Core +- **Confidence:** 90% + +Static counters modified from multiple threads without atomics. Statistics incorrect, potential corruption. + +--- + +### HIGH-011: Use-After-Free in Miner Shutdown Path +- **File:** `src/proxy/Miner.cpp:547-577` +- **Domain:** Proxy Core +- **Confidence:** 85% + +Complex callback chain. Miner can be accessed after removal from storage if shutdowns overlap. + +--- + +### HIGH-012: Integer Overflow in ExtraNonce Allocation +- **File:** `src/proxy/splitters/extra_nonce/ExtraNonceStorage.cpp:37,99` +- **Domain:** Splitter +- **Confidence:** 90% + +`m_extraNonce` increments forever, but only 32 bits used. Nonce collision after 4B connections. + +--- + +### HIGH-013: Race Condition in NonceStorage::remove() +- **File:** `src/proxy/splitters/nicehash/NonceStorage.cpp:103-110,122-126` +- **Domain:** Splitter +- **Confidence:** 85% + +Dead slots only cleared during setJob from same client. Different clients = slots never recycled. + +--- + +### HIGH-014: Potential Use-After-Free in submitCtx() +- **File:** `src/proxy/splitters/nicehash/NonceMapper.cpp:264-278` +- **Domain:** Splitter +- **Confidence:** 85% + +Miner lookup after context retrieval. Redundant map lookup, miner may have disconnected. + +--- + +### HIGH-015: Timing Attack in Certificate Fingerprint +- **File:** `src/base/net/stratum/Tls.cpp:186` +- **Domain:** Network +- **Confidence:** 85% + +`strncasecmp()` for fingerprint comparison. Timing attack vulnerability. + +--- + +### HIGH-016: Buffer Overflow Risk in LineReader +- **File:** `src/base/net/tools/LineReader.cpp:57-71` +- **Domain:** Network +- **Confidence:** 85% + +Silently drops oversized messages without error. Protocol desync, DoS vector. + +--- + +### HIGH-017: Weak TLS Configuration - Missing Modern Options +- **File:** `src/base/net/stratum/Tls.cpp:47` +- **Domain:** Network +- **Confidence:** 80% + +Only disables SSLv2/SSLv3. TLS 1.0/1.1 still allowed (deprecated, vulnerable). + +--- + +### HIGH-018: SOCKS5 Protocol Validation Insufficient +- **File:** `src/base/net/stratum/Socks5.cpp:29-48` +- **Domain:** Network +- **Confidence:** 82% + +Accesses `data[0]`, `data[1]` without buffer length check. Malicious SOCKS5 proxy can crash. + +--- + +### HIGH-019: Race Condition in DNS Resolution +- **File:** `src/base/net/dns/DnsUvBackend.cpp:74-91` +- **Domain:** Network +- **Confidence:** 80% + +Multiple resolution requests race on shared state. Inconsistent results possible. + +--- + +### HIGH-020: No HTTP Request Body Size Limit +- **File:** `src/base/net/http/HttpContext.cpp:261` +- **Domain:** HTTP API +- **Confidence:** 95% + +Body appended without limit. Memory exhaustion via large POST. + +--- + +### HIGH-021: No HTTP Connection Limits +- **File:** `src/base/net/tools/TcpServer.cpp:71` +- **Domain:** HTTP API +- **Confidence:** 90% + +Unlimited connections accepted. Connection exhaustion DoS. + +--- + +### HIGH-022: No HTTP Request Timeout +- **File:** `src/base/net/http/HttpServer.cpp:43-59` +- **Domain:** HTTP API +- **Confidence:** 90% + +No timeout on requests. Slowloris attack vector. + +--- + +### HIGH-023: Memory Leak in BindHost Parsing +- **File:** `src/proxy/BindHost.cpp:108-112,132-135,154-157` +- **Domain:** TLS & Workers +- **Confidence:** 85% + +Raw `new char[]` not freed if String copies instead of taking ownership. + +--- + +### HIGH-024: File Descriptor Leak on Error Path +- **File:** `src/base/io/log/FileLogWriter.cpp:75-84` +- **Domain:** Base I/O +- **Confidence:** 90% + +If `uv_fs_open` succeeds but check fails, fd leaked (set to -1 without close). + +--- + +### HIGH-025: Race Condition in FileLogWriter Async Flush +- **File:** `src/base/io/log/FileLogWriter.cpp:138-152` +- **Domain:** Base I/O +- **Confidence:** 88% + +`m_pos` updated before async write completes. Out-of-order writes corrupt log. + +--- --- ## Medium Priority Issues -_Pending review..._ +### MED-001: Windows Background Mode Closes Invalid Handle +- **File:** `src/App_win.cpp:44-45` +- **Domain:** Entry Point +- **Confidence:** 90% + +`CloseHandle()` on standard handle - should not be closed manually. + +--- + +### MED-002: Resource Leaks on Early Return Paths +- **File:** `src/App.cpp:46-74` +- **Domain:** Entry Point +- **Confidence:** 85% + +Multiple return paths leave UV handles partially initialized without cleanup. + +--- + +### MED-003: Config Reload Race Condition +- **File:** `src/base/kernel/Base.cpp:254-279,296-313` +- **Domain:** Config +- **Confidence:** 80% + +Config swapped without synchronization. Concurrent readers may access freed config. + +--- + +### MED-004: Integer Overflow in Miner::setJob() +- **File:** `src/proxy/Miner.cpp:154` +- **Domain:** Proxy Core +- **Confidence:** 80% + +Division by zero if `m_customDiff` is 0. + +--- + +### MED-005: Buffer Overflow Risk in Workers Name Display +- **File:** `src/proxy/workers/Workers.cpp:96` +- **Domain:** TLS & Workers +- **Confidence:** 80% + +Complex memcpy arithmetic for name truncation. Off-by-one potential. + +--- + +### MED-006: Unbounded Memory in TickingCounter +- **File:** `src/proxy/TickingCounter.h:60,64` +- **Domain:** TLS & Workers +- **Confidence:** 85% + +`m_data` vector grows unbounded with each tick(). + +--- + +### MED-007: Static Buffer in TLS Read - Thread Safety +- **File:** `src/base/net/stratum/Tls.cpp:130-135` +- **Domain:** Network +- **Confidence:** 85% + +Static buffer shared across all TLS instances. Data corruption possible. + +--- + +### MED-008: Overly Permissive CORS Configuration +- **File:** `src/base/net/http/HttpApiResponse.cpp:53-55` +- **Domain:** HTTP API +- **Confidence:** 85% + +`Access-Control-Allow-Origin: *` allows any website to access API. + +--- + +### MED-009: TLS 1.0/1.1 Support - Deprecated Protocols +- **File:** `src/base/net/tls/TlsContext.cpp:152,271-279` +- **Domain:** HTTP API +- **Confidence:** 85% + +Deprecated TLS versions not disabled by default. Downgrade attacks possible. + +--- + +### MED-010: Cipher Suite Error Ignored +- **File:** `src/base/net/tls/TlsContext.cpp:165-174` +- **Domain:** HTTP API +- **Confidence:** 82% + +Same as CRIT-009, duplicate finding confirming severity. + +--- + +### MED-011: Integer Overflow in Keccak +- **File:** `src/base/crypto/keccak.cpp:176,190-191` +- **Domain:** Base I/O +- **Confidence:** 82% + +`rsiz` calculation can underflow with large `mdlen`. + +--- + +### MED-012: Missing Null Check in Console +- **File:** `src/base/io/Console.cpp:33-40,74` +- **Domain:** Base I/O +- **Confidence:** 85% + +`m_listener` not null-checked in callbacks. + +--- + +### MED-013: TOCTOU in Watcher +- **File:** `src/base/io/Watcher.cpp:74-82` +- **Domain:** Base I/O +- **Confidence:** 80% + +File can be replaced between callback and restart. Acceptable for config files. --- ## Recommended Priority Order ### Immediate (Security Critical) -_Pending review..._ +1. CRIT-014: Timing attack in API authentication +2. CRIT-013: Missing TLS certificate verification +3. CRIT-001: Double-delete in Controller +4. CRIT-005: MemPool thread safety +5. CRIT-015: Signal handler race condition ### This Week (Data Integrity) -_Pending review..._ +6. CRIT-004: Events system race condition +7. CRIT-006: Static event buffer corruption +8. CRIT-010: Unbounded m_results memory +9. HIGH-007: StatsData unbounded memory +10. HIGH-020: HTTP body size limit ### Next Sprint (Stability) -_Pending review..._ +11. CRIT-002: UV loop cleanup +12. CRIT-011: gc() out-of-bounds access +13. CRIT-012: SSL_write return checking +14. HIGH-021: Connection limits +15. HIGH-022: Request timeouts ### Backlog (Quality) -_Pending review..._ +- All Medium priority items +- Documentation updates +- Performance optimizations --- ## Review Completion Status -- [ ] Entry Point & App Lifecycle - Pending -- [ ] Core Controller & Config - Pending -- [ ] Proxy Core - Pending -- [ ] Proxy TLS & Workers - Pending -- [ ] Splitter System - Pending -- [ ] Network & Stratum Client - Pending -- [ ] HTTP/HTTPS & REST API - Pending -- [ ] Base I/O & Kernel - Pending +- [x] Entry Point & App Lifecycle - 6 issues found +- [x] Core Controller & Config - 6 issues found +- [x] Proxy Core (Server, Miner, Events) - 10 issues found +- [x] Proxy TLS & Workers - 7 issues found +- [x] Splitter System - 5 issues found +- [x] Network & Stratum Client - 9 issues found +- [x] HTTP/HTTPS & REST API - 7 issues found +- [x] Base I/O & Kernel - 7 issues found -**Total Issues Identified: TBD** +**Total Issues Identified: 57** + +--- + +## Files Requiring Immediate Attention + +1. `src/core/Controller.cpp` - Double-delete, null checks +2. `src/base/api/Httpd.cpp` - Timing attack +3. `src/base/net/tls/TlsContext.cpp` - Cipher error, TLS config +4. `src/base/net/tools/MemPool.h` - Thread safety +5. `src/proxy/Events.cpp` - Race condition +6. `src/proxy/events/Event.h` - Static buffer +7. `src/base/io/Signals.cpp` - Async-signal-safety +8. `src/base/net/stratum/Tls.cpp` - SSL_write, cert verify +9. `src/proxy/splitters/nicehash/NonceSplitter.cpp` - Bounds check +10. `src/base/net/http/HttpContext.cpp` - Body size limit diff --git a/miner/proxy/src/donate.h b/miner/proxy/src/donate.h index 2d33883..4479d8d 100644 --- a/miner/proxy/src/donate.h +++ b/miner/proxy/src/donate.h @@ -1,12 +1,6 @@ -/* XMRig - * Copyright 2010 Jeff Garzik - * Copyright 2012-2014 pooler - * Copyright 2014 Lucas Jones - * Copyright 2014-2016 Wolf9466 - * Copyright 2016 Jay D Dee - * Copyright 2017-2018 XMR-Stak , - * Copyright 2018-2019 SChernykh - * Copyright 2016-2019 XMRig , +/* Miner Platform - Proxy + * Copyright (c) 2025 Lethean + * Based on XMRig-Proxy by SChernykh and XMRig team (GPL-3.0-or-later) * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -22,31 +16,21 @@ * along with this program. If not, see . */ -#ifndef XMRIG_DONATE_H -#define XMRIG_DONATE_H +#ifndef MINER_DONATE_H +#define MINER_DONATE_H #include /* - * Dev donation. + * Dev donation disabled. * - * Percentage of your hashing power that you want to donate to the developer. - * This helps fund ongoing open source development. - * - * XMR: 89qpYgfAZzp8VYKaPbAh1V2vSW9RHCMyHVQxe2oFxZvpK9dF1UMpZSxJK9jikW4QCRGgVni8BJjvTQpJQtHJzYyw8Uz18An - * - * How it works: - * Upstreams randomly switch to dev pool in range from 50 to 150 minutes, to reduce dev pool peak load. - * Stays on dev pool at least kDonateLevel minutes. - * Choice next donation time, with overtime compensation. In proxy no way to use precise donation time. - * You can check actual donation via API. - * - * If you set level to 0 it will enable donate over proxy feature. + * Miner Platform does not include any built-in developer fees. + * If you'd like to support development, please visit https://lethean.io */ constexpr const int kDefaultDonateLevel = 0; constexpr const int kMinimumDonateLevel = 0; -#endif /* XMRIG_DONATE_H */ +#endif /* MINER_DONATE_H */ diff --git a/miner/proxy/src/xmrig.cpp b/miner/proxy/src/miner.cpp similarity index 100% rename from miner/proxy/src/xmrig.cpp rename to miner/proxy/src/miner.cpp diff --git a/miner/proxy/src/version.h b/miner/proxy/src/version.h index 83f310c..d4a3575 100644 --- a/miner/proxy/src/version.h +++ b/miner/proxy/src/version.h @@ -1,6 +1,6 @@ -/* XMRig - * Copyright (c) 2018-2025 SChernykh - * Copyright (c) 2016-2025 XMRig , +/* Miner Platform - Proxy + * Copyright (c) 2025 Lethean + * Based on XMRig-Proxy by SChernykh and XMRig team (GPL-3.0-or-later) * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -16,8 +16,8 @@ * along with this program. If not, see . */ -#ifndef XMRIG_VERSION_H -#define XMRIG_VERSION_H +#ifndef MINER_VERSION_H +#define MINER_VERSION_H #define APP_ID "miner-proxy" #define APP_NAME "Miner Proxy" @@ -87,4 +87,4 @@ # define APP_BITS "32 bit" #endif -#endif // XMRIG_VERSION_H +#endif // MINER_VERSION_H diff --git a/pkg/mining/auth.go b/pkg/mining/auth.go index 49a6b4b..1f670be 100644 --- a/pkg/mining/auth.go +++ b/pkg/mining/auth.go @@ -169,8 +169,8 @@ func (da *DigestAuth) validateDigest(c *gin.Context, authHeader string) bool { return false } - // Validate username - if params["username"] != da.config.Username { + // Validate username with constant-time comparison to prevent timing attacks + if subtle.ConstantTimeCompare([]byte(params["username"]), []byte(da.config.Username)) != 1 { return false } diff --git a/pkg/mining/events.go b/pkg/mining/events.go index 49f9b3c..572915f 100644 --- a/pkg/mining/events.go +++ b/pkg/mining/events.go @@ -64,6 +64,13 @@ type wsClient struct { closeOnce sync.Once } +// safeClose closes the send channel exactly once to prevent panic on double close +func (c *wsClient) safeClose() { + c.closeOnce.Do(func() { + close(c.send) + }) +} + // StateProvider is a function that returns the current state for sync type StateProvider func() interface{} @@ -128,7 +135,7 @@ func (h *EventHub) Run() { // Close all client connections h.mu.Lock() for client := range h.clients { - close(client.send) + client.safeClose() delete(h.clients, client) } h.mu.Unlock() @@ -174,7 +181,7 @@ func (h *EventHub) Run() { h.mu.Lock() if _, ok := h.clients[client]; ok { delete(h.clients, client) - close(client.send) + client.safeClose() } h.mu.Unlock() logging.Debug("client disconnected", logging.Fields{"total": len(h.clients)}) diff --git a/pkg/mining/manager_test.go b/pkg/mining/manager_test.go index 84bde46..94a46f8 100644 --- a/pkg/mining/manager_test.go +++ b/pkg/mining/manager_test.go @@ -13,7 +13,7 @@ import ( // It also temporarily modifies the PATH to include the dummy executable's directory. func setupTestManager(t *testing.T) *Manager { dummyDir := t.TempDir() - executableName := "xmrig" + executableName := "miner" if runtime.GOOS == "windows" { executableName += ".exe" } diff --git a/pkg/mining/service.go b/pkg/mining/service.go index 690da41..2bdd19c 100644 --- a/pkg/mining/service.go +++ b/pkg/mining/service.go @@ -118,6 +118,55 @@ func isRetryableError(status int) bool { status == http.StatusGatewayTimeout } +// securityHeadersMiddleware adds security headers to all responses. +// This helps protect against common web vulnerabilities. +func securityHeadersMiddleware() gin.HandlerFunc { + return func(c *gin.Context) { + // Prevent MIME type sniffing + c.Header("X-Content-Type-Options", "nosniff") + // Prevent clickjacking + c.Header("X-Frame-Options", "DENY") + // Enable XSS filter in older browsers + c.Header("X-XSS-Protection", "1; mode=block") + // Restrict referrer information + c.Header("Referrer-Policy", "strict-origin-when-cross-origin") + // Content Security Policy for API responses + c.Header("Content-Security-Policy", "default-src 'none'; frame-ancestors 'none'") + c.Next() + } +} + +// contentTypeValidationMiddleware ensures POST/PUT requests have proper Content-Type. +func contentTypeValidationMiddleware() gin.HandlerFunc { + return func(c *gin.Context) { + method := c.Request.Method + if method != http.MethodPost && method != http.MethodPut && method != http.MethodPatch { + c.Next() + return + } + + // Skip if no body expected + if c.Request.ContentLength == 0 { + c.Next() + return + } + + contentType := c.GetHeader("Content-Type") + // Allow JSON and form data + if strings.HasPrefix(contentType, "application/json") || + strings.HasPrefix(contentType, "application/x-www-form-urlencoded") || + strings.HasPrefix(contentType, "multipart/form-data") { + c.Next() + return + } + + respondWithError(c, http.StatusUnsupportedMediaType, ErrCodeInvalidInput, + "Unsupported Content-Type", + "Use application/json for API requests") + c.Abort() + } +} + // requestIDMiddleware adds a unique request ID to each request for tracing func requestIDMiddleware() gin.HandlerFunc { return func(c *gin.Context) { @@ -448,6 +497,12 @@ func (s *Service) InitRouter() { } s.Router.Use(cors.New(corsConfig)) + // Add security headers (SEC-LOW-4) + s.Router.Use(securityHeadersMiddleware()) + + // Add Content-Type validation for POST/PUT (API-MED-8) + s.Router.Use(contentTypeValidationMiddleware()) + // Add request body size limit middleware (1MB max) s.Router.Use(func(c *gin.Context) { c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, 1<<20) // 1MB diff --git a/pkg/mining/xmrig.go b/pkg/mining/xmrig.go index dd230b4..39f7c0e 100644 --- a/pkg/mining/xmrig.go +++ b/pkg/mining/xmrig.go @@ -49,17 +49,19 @@ func setHTTPClient(client *http.Client) { } // MinerTypeXMRig is the type identifier for XMRig miners. +// Note: This type now supports the Miner Platform binary ("miner") as the default. const MinerTypeXMRig = "xmrig" // NewXMRigMiner creates a new XMRig miner instance with default settings. +// The executable name defaults to "miner" (Miner Platform) but can also find "xmrig". func NewXMRigMiner() *XMRigMiner { return &XMRigMiner{ BaseMiner: BaseMiner{ - Name: "xmrig", + Name: "miner", MinerType: MinerTypeXMRig, - ExecutableName: "xmrig", + ExecutableName: "miner", Version: "latest", - URL: "https://github.com/xmrig/xmrig/releases", + URL: "", // Local build only - no remote download API: &API{ Enabled: true, ListenHost: "127.0.0.1", diff --git a/pkg/mining/xmrig_start.go b/pkg/mining/xmrig_start.go index bb121a6..2d0f9f4 100644 --- a/pkg/mining/xmrig_start.go +++ b/pkg/mining/xmrig_start.go @@ -64,7 +64,7 @@ func (m *XMRigMiner) Start(config *Config) error { addCliArgs(config, &args) - logging.Info("executing XMRig command", logging.Fields{"binary": m.MinerBinary, "args": strings.Join(args, " ")}) + logging.Info("executing miner command", logging.Fields{"binary": m.MinerBinary, "args": strings.Join(args, " ")}) m.cmd = exec.Command(m.MinerBinary, args...) diff --git a/pkg/mining/xmrig_test.go b/pkg/mining/xmrig_test.go index 8504627..f032fed 100644 --- a/pkg/mining/xmrig_test.go +++ b/pkg/mining/xmrig_test.go @@ -45,8 +45,8 @@ func TestNewXMRigMiner_Good(t *testing.T) { if miner == nil { t.Fatal("NewXMRigMiner returned nil") } - if miner.Name != "xmrig" { - t.Errorf("Expected miner name to be 'xmrig', got '%s'", miner.Name) + if miner.Name != "miner" { + t.Errorf("Expected miner name to be 'miner', got '%s'", miner.Name) } if miner.Version != "latest" { t.Errorf("Expected miner version to be 'latest', got '%s'", miner.Version) @@ -58,8 +58,8 @@ func TestNewXMRigMiner_Good(t *testing.T) { func TestXMRigMiner_GetName_Good(t *testing.T) { miner := NewXMRigMiner() - if name := miner.GetName(); name != "xmrig" { - t.Errorf("Expected GetName() to return 'xmrig', got '%s'", name) + if name := miner.GetName(); name != "miner" { + t.Errorf("Expected GetName() to return 'miner', got '%s'", name) } } diff --git a/pkg/node/message.go b/pkg/node/message.go index 77032c7..6f343df 100644 --- a/pkg/node/message.go +++ b/pkg/node/message.go @@ -7,6 +7,28 @@ import ( "github.com/google/uuid" ) +// Protocol version constants +const ( + // ProtocolVersion is the current protocol version + ProtocolVersion = "1.0" + // MinProtocolVersion is the minimum supported version + MinProtocolVersion = "1.0" +) + +// SupportedProtocolVersions lists all protocol versions this node supports. +// Used for version negotiation during handshake. +var SupportedProtocolVersions = []string{"1.0"} + +// IsProtocolVersionSupported checks if a given version is supported. +func IsProtocolVersionSupported(version string) bool { + for _, v := range SupportedProtocolVersions { + if v == version { + return true + } + } + return false +} + // MessageType defines the type of P2P message. type MessageType string diff --git a/pkg/node/peer.go b/pkg/node/peer.go index 994fb06..a7937a3 100644 --- a/pkg/node/peer.go +++ b/pkg/node/peer.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "regexp" "sync" "time" @@ -46,6 +47,34 @@ const ( PeerAuthAllowlist ) +// Peer name validation constants +const ( + PeerNameMinLength = 1 + PeerNameMaxLength = 64 +) + +// peerNameRegex validates peer names: alphanumeric, hyphens, underscores, and spaces +var peerNameRegex = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9\-_ ]{0,62}[a-zA-Z0-9]$|^[a-zA-Z0-9]$`) + +// validatePeerName checks if a peer name is valid. +// Peer names must be 1-64 characters, start and end with alphanumeric, +// and contain only alphanumeric, hyphens, underscores, and spaces. +func validatePeerName(name string) error { + if name == "" { + return nil // Empty names are allowed (optional field) + } + if len(name) < PeerNameMinLength { + return fmt.Errorf("peer name too short (min %d characters)", PeerNameMinLength) + } + if len(name) > PeerNameMaxLength { + return fmt.Errorf("peer name too long (max %d characters)", PeerNameMaxLength) + } + if !peerNameRegex.MatchString(name) { + return fmt.Errorf("peer name contains invalid characters (use alphanumeric, hyphens, underscores, spaces)") + } + return nil +} + // PeerRegistry manages known peers with KD-tree based selection. type PeerRegistry struct { peers map[string]*Peer @@ -54,9 +83,9 @@ type PeerRegistry struct { mu sync.RWMutex // Authentication settings - authMode PeerAuthMode // How to handle unknown peers - allowedPublicKeys map[string]bool // Allowlist of public keys (when authMode is Allowlist) - allowedPublicKeyMu sync.RWMutex // Protects allowedPublicKeys + authMode PeerAuthMode // How to handle unknown peers + allowedPublicKeys map[string]bool // Allowlist of public keys (when authMode is Allowlist) + allowedPublicKeyMu sync.RWMutex // Protects allowedPublicKeys // Debounce disk writes dirty bool // Whether there are unsaved changes @@ -196,6 +225,12 @@ func (r *PeerRegistry) AddPeer(peer *Peer) error { return fmt.Errorf("peer ID is required") } + // Validate peer name (P2P-LOW-3) + if err := validatePeerName(peer.Name); err != nil { + r.mu.Unlock() + return err + } + if _, exists := r.peers[peer.ID]; exists { r.mu.Unlock() return fmt.Errorf("peer %s already exists", peer.ID) diff --git a/pkg/node/transport.go b/pkg/node/transport.go index 3c8e9f1..856c993 100644 --- a/pkg/node/transport.go +++ b/pkg/node/transport.go @@ -161,8 +161,8 @@ type PeerConnection struct { LastActivity time.Time writeMu sync.Mutex // Serialize WebSocket writes transport *Transport - closeOnce sync.Once // Ensure Close() is only called once - rateLimiter *PeerRateLimiter // Per-peer message rate limiting + closeOnce sync.Once // Ensure Close() is only called once + rateLimiter *PeerRateLimiter // Per-peer message rate limiting } // NewTransport creates a new WebSocket transport. @@ -343,11 +343,16 @@ func (t *Transport) Send(peerID string, msg *Message) error { return pc.Send(msg) } -// Broadcast sends a message to all connected peers. +// Broadcast sends a message to all connected peers except the sender. +// The sender is identified by msg.From and excluded to prevent echo. func (t *Transport) Broadcast(msg *Message) error { t.mu.RLock() conns := make([]*PeerConnection, 0, len(t.conns)) for _, pc := range t.conns { + // Exclude sender from broadcast to prevent echo (P2P-MED-6) + if pc.Peer != nil && pc.Peer.ID == msg.From { + continue + } conns = append(conns, pc) } t.mu.RUnlock() @@ -427,6 +432,29 @@ func (t *Transport) handleWSUpgrade(w http.ResponseWriter, r *http.Request) { return } + // Check protocol version compatibility (P2P-MED-1) + if !IsProtocolVersionSupported(payload.Version) { + logging.Warn("peer connection rejected: incompatible protocol version", logging.Fields{ + "peer_version": payload.Version, + "supported_versions": SupportedProtocolVersions, + "peer_id": payload.Identity.ID, + }) + identity := t.node.GetIdentity() + if identity != nil { + rejectPayload := HandshakeAckPayload{ + Identity: *identity, + Accepted: false, + Reason: fmt.Sprintf("incompatible protocol version %s, supported: %v", payload.Version, SupportedProtocolVersions), + } + rejectMsg, _ := NewMessage(MsgHandshakeAck, identity.ID, payload.Identity.ID, rejectPayload) + if rejectData, err := MarshalJSON(rejectMsg); err == nil { + conn.WriteMessage(websocket.TextMessage, rejectData) + } + } + conn.Close() + return + } + // Derive shared secret from peer's public key sharedSecret, err := t.node.DeriveSharedSecret(payload.Identity.PublicKey) if err != nil { @@ -566,7 +594,7 @@ func (t *Transport) performHandshake(pc *PeerConnection) error { payload := HandshakePayload{ Identity: *identity, Challenge: challenge, - Version: "1.0", + Version: ProtocolVersion, } msg, err := NewMessage(MsgHandshake, identity.ID, pc.Peer.ID, payload)