← Watch feed

Low 46 Cryptographic libraries Committed Sep 28, 2026 · 18:22 UTC

Merge pull request #11369

XMR

Monero Project/Monero

3b7d8421 View source ↗

Public commit record

What the developer wrote

Authored by tobtoht

63/100 · Adequate

Merge pull request #11369

e521789 crypto: random and siphash static-init-safety (jeffro256)

ACKs: jpk68, selsta, thomasbuilds

✓ Descriptive subject✓ Provides an explanatory body✓ Links an issue, advisory, or supporting reference✓ Names security-relevant behavior explicitly

The short version

What changed, and why it matters

This Monero commit changes how sensitive random-number and hash-key setup is performed. Previously, some initialization code could run automatically during program startup in a way that wasn't guaranteed to be thread-safe, and the secret SipHash key was directly accessible as a global variable. The patch makes initialization happen on first use using platform-native 'run once' primitives, splits the SipHash key initialization into its own guarded routine, and hides the key behind an accessor function. These are defensive hardening improvements; the commit message does not frame them as fixing an active vulnerability.

Recommended action

Treat as a defensive hardening patch. Review whether any code paths still read crypto_siphash_key directly (now removed from the header) and ensure all consumers use get_static_siphash_key(). Verify that CTHR_ONCE_CALL error handling and assertion behavior are appropriate for production builds. Consider whether the change warrants a release note or advisory if prior Monero versions had a practical race condition in RNG initialization.

Security signals we found

Static initialization order fiasco mitigation

Thread-safe one-shot initialization for RNG state

SipHash key no longer exposed as writable global array

Accessor function added for secret SipHash key

Separate finalizers for RNG state and SipHash key

Test harness updated to ensure deterministic initialization ordering

Technical analysis

Evidence from the diff

The patch refactors src/crypto/random.c and related headers: (1) replaces compiler/linker INITIALIZER/FINALIZER-based random initialization with pthread_once/InitOnceExecuteOnce one-shot initialization triggered on first use of generate_random_bytes_not_thread_safe() or add_extra_entropy_not_thread_safe(); (2) moves SipHash key generation into a separate get_static_siphash_key() accessor protected by its own once-flag, and changes generic-ops.h to call that accessor instead of reading the global crypto_siphash_key array; (3) makes crypto_siphash_key static and removes it from the public header; (4) adds CTHR_ONCE_* macros in c_threads.h for Windows and POSIX; (5) updates tests/crypto/random.c to force initialization before test state setup. The change addresses static-init-order and thread-safety concerns around RNG and SipHash key setup, but the commit itself is presented as hardening rather than a security bug fix.

Changed components

src/crypto/random.csrc/crypto/random.hsrc/crypto/generic-ops.hsrc/crypto/c_threads.htests/crypto/random.c

Inspect captured patch +62 / −10

### src/crypto/c_threads.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2019-2024, The Monero Project
+// Copyright (c) 2019-2026, The Monero Project
 //
 // All rights reserved.
 //
@@ -48,6 +48,12 @@
 #define CTHR_THREAD_JOIN(thr)			do { WaitForSingleObject(thr, INFINITE); CloseHandle(thr); } while(0)
 #define CTHR_THREAD_CLOSE(thr)			CloseHandle((HANDLE)thr);
 
+#define CTHR_ONCE_FLAG             INIT_ONCE
+#define CTHR_ONCE_INIT             INIT_ONCE_STATIC_INIT
+#define CTHR_ONCE_CALL(flag, f)    (0 != InitOnceExecuteOnce(flag, f, NULL, NULL))
+#define CTHR_ONCE_DECLARE_CB(name) BOOL CALLBACK name(PINIT_ONCE InitOnce, PVOID Parameter, PVOID *Context)
+#define CTHR_ONCE_CB_SUCCESS       true
+
 #else
 
 #include <pthread.h>
@@ -67,4 +73,10 @@
 #define CTHR_THREAD_JOIN(thr)			pthread_join(thr, NULL)
 #define CTHR_THREAD_CLOSE(thr)			pthread_detach(thr)
 
+#define CTHR_ONCE_FLAG             pthread_once_t
+#define CTHR_ONCE_INIT             PTHREAD_ONCE_INIT
+#define CTHR_ONCE_CALL(flag, f)    (0 == pthread_once(flag, f))
+#define CTHR_ONCE_DECLARE_CB(name) void name(void)
+#define CTHR_ONCE_CB_SUCCESS
+
 #endif

### src/crypto/generic-ops.h
@@ -73,12 +73,13 @@ namespace crypto { \
 
 namespace crypto {
   inline std::size_t siphash_to_size_t(const void *data, std::size_t length) {
-    static_assert(sizeof(crypto_siphash_key) == crypto_shorthash_siphash24_KEYBYTES,
+    static_assert(16 == crypto_shorthash_siphash24_KEYBYTES,
       "crypto_siphash_key size must match the SipHash-2-4 key length");
     static_assert(sizeof(std::uint64_t) == crypto_shorthash_siphash24_BYTES,
       "std::uint64_t size must match the SipHash-2-4 digest length");
     std::uint64_t h;
-    crypto_shorthash_siphash24(reinterpret_cast<unsigned char*>(&h), static_cast<const unsigned char*>(data), length, crypto_siphash_key);
+    crypto_shorthash_siphash24(reinterpret_cast<unsigned char*>(&h), static_cast<const unsigned char*>(data), length,
+      get_static_siphash_key());
     return h;
   }
 }

### src/crypto/random.c
@@ -1,4 +1,4 @@
-// Copyright (c) 2014-2024, The Monero Project
+// Copyright (c) 2014-2026, The Monero Project
 // 
 // All rights reserved.
 // 
@@ -32,6 +32,7 @@
 #include <stddef.h>
 #include <string.h>
 
+#include "c_threads.h"
 #include "hash-ops.h"
 #include "initializer.h"
 #include "random.h"
@@ -97,18 +98,22 @@ static void generate_system_random_bytes(size_t n, void *result) {
 #endif
 
 static union hash_state state;
-unsigned char crypto_siphash_key[16];
+static unsigned char crypto_siphash_key[16];
 
 #if !defined(NDEBUG)
 static volatile int curstate; /* To catch thread safety problems. */
 #endif
+static CTHR_ONCE_FLAG init_random_once = CTHR_ONCE_INIT;
 
 FINALIZER(deinit_random) {
 #if !defined(NDEBUG)
-  assert(curstate == 1);
+  assert(curstate == 1 || curstate == 0);
   curstate = 0;
 #endif
   memset(&state, 0, sizeof(union hash_state));
+}
+
+FINALIZER(deinit_sip) {
   memset(crypto_siphash_key, 0, sizeof(crypto_siphash_key));
 }
 
@@ -122,19 +127,20 @@ static void lock_random_state(void) {
 
 #define HASH_CAPACITY_AREA 64
 
-INITIALIZER(init_random) {
+static CTHR_ONCE_DECLARE_CB(init_random) {
   static_assert(sizeof(state) == HASH_DATA_AREA + HASH_CAPACITY_AREA,
     "Keccak state is the wrong size");
   lock_random_state();
   generate_system_random_bytes(HASH_DATA_AREA, &state);
   hash_permutation(&state);
 
-  generate_system_random_bytes(sizeof(crypto_siphash_key), crypto_siphash_key);
   REGISTER_FINALIZER(deinit_random);
 #if !defined(NDEBUG)
   assert(curstate == 0);
   curstate = 1;
 #endif
+
+  return CTHR_ONCE_CB_SUCCESS;
 }
 
 void generate_random_bytes_not_thread_safe(size_t n, void *result) {
@@ -144,6 +150,9 @@ void generate_random_bytes_not_thread_safe(size_t n, void *result) {
    * leaving this function does not leak the previous squeezed values.
    */
 
+  const int r = CTHR_ONCE_CALL(&init_random_once, init_random);
+  (void) r; assert(r);
+
 #if !defined(NDEBUG)
   assert(curstate == 1);
   curstate = 2;
@@ -171,6 +180,14 @@ void generate_random_bytes_not_thread_safe(size_t n, void *result) {
 
 void add_extra_entropy_not_thread_safe(const void *ptr, size_t bytes)
 {
+  const int r = CTHR_ONCE_CALL(&init_random_once, init_random);
+  (void) r; assert(r);
+
+#if !defined(NDEBUG)
+  assert(curstate == 1);
+  curstate = 2;
+#endif
+
   size_t i;
 
   while (bytes > 0)
@@ -182,4 +199,24 @@ void add_extra_entropy_not_thread_safe(const void *ptr, size_t bytes)
     ptr = cpadd(ptr, round_bytes);
     hash_permutation(&state);
   }
+
+#if !defined(NDEBUG)
+  assert(curstate == 2);
+  curstate = 1;
+#endif
+}
+
+static CTHR_ONCE_DECLARE_CB(init_sip)
+{
+  generate_system_random_bytes(sizeof(crypto_siphash_key), crypto_siphash_key);
+  REGISTER_FINALIZER(deinit_sip);
+  return CTHR_ONCE_CB_SUCCESS;
+}
+
+const unsigned char *get_static_siphash_key(void)
+{
+  static CTHR_ONCE_FLAG once = CTHR_ONCE_INIT;
+  const int r = CTHR_ONCE_CALL(&once, init_sip);
+  (void) r; assert(r);
+  return crypto_siphash_key;
 }

### src/crypto/random.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2014-2024, The Monero Project
+// Copyright (c) 2014-2026, The Monero Project
 // 
 // All rights reserved.
 // 
@@ -39,7 +39,8 @@ extern "C" {
 void generate_random_bytes_not_thread_safe(size_t n, void *result);
 void add_extra_entropy_not_thread_safe(const void *ptr, size_t bytes);
 
-extern unsigned char crypto_siphash_key[16];
+//! @brief Return pointer to random, constant 16-byte key, thread-safe and static-init-safe
+const unsigned char *get_static_siphash_key(void);
 
 #ifdef __cplusplus
 }

### tests/crypto/random.c
@@ -33,6 +33,7 @@
 #include "crypto-tests.h"
 
 void setup_random(void) {
+    generate_random_bytes_not_thread_safe(0, NULL);
     memset(&state, 42, sizeof(union hash_state));
     hash_permutation(&state);
 }

Risk score

Why this scored 46/100

Our methodology →

Potential impact 12/30

Exploitability 8/25

Stealth signal 7/15

Affected reach 10/15

Confidence 6/10

Evidence quality 3/5

Human-validated context

Community notes

Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.

No validated notes yet.

The AI analysis stands alone for now. Submit a note if you can add evidence or important context.

What the developer wrote

What changed, and why it matters

Security signals we found

Evidence from the diff

Changed components

Why this scored 46/100

Community notes

Related watches

Merge pull request #11462

Merge pull request #11460

wallet2: trim stale transfer maps after output imports