fix: Address 9 additional security findings in proxy (batch 2)

Critical fixes:
- CRIT-008: Enable TLS peer certificate verification in client connections
- CRIT-010: Add null pointer checks throughout ExtraNonceSplitter
- CRIT-011: Validate JSON error message field before access in Client
- CRIT-013: Make event system thread-safe with atomic<bool> and CAS
- CRIT-018: Add bounds checking in NonceSplitter vector access

High priority fixes:
- HIGH-002: Set 0600 permissions on generated private key files
- HIGH-004: Add strtol error checking and overflow validation
- HIGH-007: Handle integer overflow in ExtraNonceStorage nonce counter
- HIGH-018: Add comprehensive JSON field validation in Miner::parse()

These fixes address TLS security, thread safety, memory safety, and
input validation issues identified during parallel code review.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

miner/proxy/src/base/net/stratum/Client.cpp

@@ -812,7 +812,11 @@ void xmrig::Client::parseResponse(int64_t id, const rapidjson::Value &result, co
     }
 
     if (error.IsObject()) {
-        const char *message = error["message"].GetString();
+        // SECURITY FIX (CRIT-011): Validate "message" field exists and is a string before accessing
+        const char *message = Json::getString(error, "message");
+        if (!message) {
+            message = "unknown error";
+        }
 
         if (!handleSubmitResponse(id, message) && !isQuiet()) {
             LOG_ERR("%s " RED("error: ") RED_BOLD("\"%s\"") RED(", code: ") RED_BOLD("%d"), tag(), message, Json::getInt(error, "code"));

miner/proxy/src/base/net/stratum/Tls.cpp

@@ -45,6 +45,10 @@ xmrig::Client::Tls::Tls(Client *client) :
     m_write = BIO_new(BIO_s_mem());
     m_read  = BIO_new(BIO_s_mem());
     SSL_CTX_set_options(m_ctx, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
+
+    // SECURITY FIX (CRIT-008): Enable peer certificate verification
+    // When no fingerprint is configured, this ensures we at least validate the certificate chain
+    SSL_CTX_set_verify(m_ctx, SSL_VERIFY_PEER, nullptr);
 }
 
 

miner/proxy/src/base/net/tls/TlsGen.cpp

@@ -24,6 +24,11 @@
 #include <stdexcept>
 #include <fstream>
 
+// SECURITY FIX (HIGH-002): Include for setting file permissions
+#ifndef _WIN32
+#   include <sys/stat.h>
+#endif
+
 
 namespace xmrig {
 
@@ -137,6 +142,11 @@ bool xmrig::TlsGen::write()
         return false;
     }
 
+    // SECURITY FIX (HIGH-002): Set restrictive permissions on private key file (owner read/write only)
+#   ifndef _WIN32
+    chmod(m_certKey, S_IRUSR | S_IWUSR);  // 0600
+#   endif
+
     auto x509_file = fopen(m_cert, "wb");
     if (!x509_file) {
         return false;

miner/proxy/src/core/config/ConfigTransform.cpp

@@ -27,6 +27,9 @@
 #include "base/kernel/interfaces/IConfig.h"
 #include "proxy/BindHost.h"
 
+#include <cerrno>
+#include <climits>
+
 
 namespace xmrig
 {
@@ -82,7 +85,16 @@ void xmrig::ConfigTransform::transform(rapidjson::Document &doc, int key, const
 
     case IConfig::CustomDiffKey: /* --custom-diff */
     case IConfig::ReuseTimeoutKey: /* --reuse-timeout */
-        return transformUint64(doc, key, static_cast<uint64_t>(strtol(arg, nullptr, 10)));
+        {
+            // SECURITY FIX (HIGH-004): Validate strtol result for errors and overflow
+            char *endptr = nullptr;
+            errno = 0;
+            const long val = strtol(arg, &endptr, 10);
+            if (endptr == arg || errno == ERANGE || val < 0) {
+                return;  // Invalid input, skip silently
+            }
+            return transformUint64(doc, key, static_cast<uint64_t>(val));
+        }
 
     case IConfig::LoginFileKey: /* --login-file */
         return set(doc, "login-file", arg);

miner/proxy/src/proxy/Events.cpp

@@ -22,13 +22,16 @@
  */
 
 
+#include <atomic>
+
 #include "base/io/log/Log.h"
 #include "proxy/Events.h"
 
 
 namespace xmrig {
 
-bool Events::m_ready = true;
+// THREAD SAFETY FIX (CRIT-013): Use atomic for thread-safe reentrancy guard
+std::atomic<bool> Events::m_ready{true};
 std::map<IEvent::Type, std::vector<IEventListener*> > Events::m_listeners;
 
 }
@@ -36,13 +39,13 @@ std::map<IEvent::Type, std::vector<IEventListener*> > Events::m_listeners;
 
 bool xmrig::Events::exec(IEvent *event)
 {
-    if (!m_ready) {
+    // THREAD SAFETY FIX (CRIT-013): Use atomic compare-and-swap for thread-safe guard
+    bool expected = true;
+    if (!m_ready.compare_exchange_strong(expected, false)) {
         LOG_ERR("failed start event %d", (int) event->type());
         return false;
     }
 
-    m_ready = false;
-
     std::vector<IEventListener*> &listeners = m_listeners[event->type()];
     for (IEventListener *listener : listeners) {
         event->isRejected() ? listener->onRejectedEvent(event) : listener->onEvent(event);
@@ -51,7 +54,7 @@ bool xmrig::Events::exec(IEvent *event)
     const bool rejected = event->isRejected();
     event->~IEvent();
 
-    m_ready = true;
+    m_ready.store(true);
     return !rejected;
 }
 

miner/proxy/src/proxy/Events.h

@@ -26,6 +26,7 @@
 #define XMRIG_EVENTS_H
 
 
+#include <atomic>
 #include <map>
 #include <vector>
 
@@ -45,7 +46,8 @@ public:
     static void subscribe(IEvent::Type type, IEventListener *listener);
 
 private:
-    static bool m_ready;
+    // THREAD SAFETY FIX (CRIT-013): Use atomic for thread-safe reentrancy guard
+    static std::atomic<bool> m_ready;
     static std::map<IEvent::Type, std::vector<IEventListener*> > m_listeners;
 };
 

miner/proxy/src/proxy/Miner.cpp

@@ -351,8 +351,19 @@ void xmrig::Miner::parse(char *line, size_t len)
         return shutdown(true);
     }
 
+    // SECURITY FIX (HIGH-018): Validate all JSON fields before accessing
+    if (!doc.HasMember("id") || !doc.HasMember("method")) {
+        return shutdown(true);
+    }
+
     const rapidjson::Value &id = doc["id"];
-    if (id.IsInt64() && parseRequest(id.GetInt64(), doc["method"].GetString(), doc["params"])) {
+    const rapidjson::Value &method = doc["method"];
+
+    if (!id.IsInt64() || !method.IsString()) {
+        return shutdown(true);
+    }
+
+    if (parseRequest(id.GetInt64(), method.GetString(), doc.HasMember("params") ? doc["params"] : rapidjson::Value())) {
         return;
     }
 

miner/proxy/src/proxy/splitters/extra_nonce/ExtraNonceSplitter.cpp

@@ -63,6 +63,10 @@ xmrig::ExtraNonceSplitter::~ExtraNonceSplitter()
 
 xmrig::Upstreams xmrig::ExtraNonceSplitter::upstreams() const
 {
+    // SECURITY FIX (CRIT-010): Add null check to prevent segfault before connect()
+    if (!m_upstream) {
+        return { 0U, 0U, 0U };
+    }
     return { m_upstream->isActive() ? 1U : 0U, m_upstream->isSuspended() ? 1U : 0U, 1U };
 }
 
@@ -76,7 +80,10 @@ void xmrig::ExtraNonceSplitter::connect()
 
 void xmrig::ExtraNonceSplitter::gc()
 {
-    m_upstream->gc();
+    // SECURITY FIX (CRIT-010): Add null check
+    if (m_upstream) {
+        m_upstream->gc();
+    }
 }
 
 
@@ -95,6 +102,10 @@ void xmrig::ExtraNonceSplitter::printConnections()
 
 void xmrig::ExtraNonceSplitter::tick(uint64_t ticks)
 {
+    // SECURITY FIX (CRIT-010): Add null check
+    if (!m_upstream) {
+        return;
+    }
     const uint64_t now = uv_now(uv_default_loop());
     m_upstream->tick(ticks, now);
 }
@@ -103,14 +114,18 @@ void xmrig::ExtraNonceSplitter::tick(uint64_t ticks)
 #ifdef APP_DEVEL
 void xmrig::ExtraNonceSplitter::printState()
 {
-    m_upstream->printState();
+    // SECURITY FIX (CRIT-010): Add null check
+    if (m_upstream) {
+        m_upstream->printState();
+    }
 }
 #endif
 
 
 void xmrig::ExtraNonceSplitter::onConfigChanged(Config *config, Config *previousConfig)
 {
-    if (config->pools() != previousConfig->pools()) {
+    // SECURITY FIX (CRIT-010): Add null check
+    if (m_upstream && config->pools() != previousConfig->pools()) {
         config->pools().print();
         m_upstream->reload(config->pools());
     }

miner/proxy/src/proxy/splitters/extra_nonce/ExtraNonceStorage.cpp

@@ -22,6 +22,8 @@
  *   along with this program. If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <climits>
+
 #include "base/io/log/Log.h"
 #include "proxy/Counters.h"
 #include "proxy/Miner.h"
@@ -34,7 +36,13 @@ bool xmrig::ExtraNonceStorage::add(Miner *miner)
 
     if (isActive()) {
         miner->setJob(m_job, m_extraNonce);
-        ++m_extraNonce;
+        // SECURITY FIX (HIGH-007): Handle overflow with wrap-around
+        if (m_extraNonce == INT64_MAX) {
+            m_extraNonce = 0;
+            LOG_WARN("ExtraNonce overflow, wrapping to 0");
+        } else {
+            ++m_extraNonce;
+        }
     }
 
     return true;
@@ -96,7 +104,13 @@ void xmrig::ExtraNonceStorage::setJob(const Job &job)
 
     for (const auto& m : m_miners) {
         m.second->setJob(m_job, m_extraNonce);
-        ++m_extraNonce;
+        // SECURITY FIX (HIGH-007): Handle overflow with wrap-around
+        if (m_extraNonce == INT64_MAX) {
+            m_extraNonce = 0;
+            LOG_WARN("ExtraNonce overflow during setJob, wrapping to 0");
+        } else {
+            ++m_extraNonce;
+        }
     }
 }
 

miner/proxy/src/proxy/splitters/nicehash/NonceSplitter.cpp

@@ -196,7 +196,13 @@ void xmrig::NonceSplitter::remove(Miner *miner)
         return;
     }
 
-    m_upstreams[miner->mapperId()]->remove(miner);
+    // SECURITY FIX (CRIT-018): Add bounds check before vector access
+    const auto mapperId = static_cast<size_t>(miner->mapperId());
+    if (mapperId >= m_upstreams.size()) {
+        return;
+    }
+
+    m_upstreams[mapperId]->remove(miner);
 }
 
 
@@ -206,5 +212,11 @@ void xmrig::NonceSplitter::submit(SubmitEvent *event)
         return;
     }
 
-    m_upstreams[event->miner()->mapperId()]->submit(event);
+    // SECURITY FIX (CRIT-018): Add bounds check before vector access
+    const auto mapperId = static_cast<size_t>(event->miner()->mapperId());
+    if (mapperId >= m_upstreams.size()) {
+        return;
+    }
+
+    m_upstreams[mapperId]->submit(event);
 }