Low-severity hardening: CSPRNG UUIDs (POSIX) and null-safe zlib error logs

lib/compression/HttpDeflateCompression.cpp

@@ -44,7 +44,7 @@ namespace MAT_NS_BEGIN {
 
         int result = deflateInit2(&stream, Z_DEFAULT_COMPRESSION, Z_DEFLATED, m_windowBits, 8 /*DEF_MEM_LEVEL*/, Z_DEFAULT_STRATEGY);
         if (result != Z_OK) {
-            LOG_WARN("HTTP request compressing failed, error=%u/%u (%s)", 1, result, stream.msg);
+            LOG_WARN("HTTP request compressing failed, error=%d/%d (%s)", 1, result, (stream.msg ? stream.msg : "(null)"));
             compressionFailed(ctx);
             return false;
         }
@@ -80,7 +80,7 @@ namespace MAT_NS_BEGIN {
         deflateEnd(&stream);
 
         if (result != Z_STREAM_END) {
-            LOG_WARN("HTTP request compressing failed, error=%u/%u (%s)", 2, result, stream.msg);
+            LOG_WARN("HTTP request compressing failed, error=%d/%d (%s)", 2, result, (stream.msg ? stream.msg : "(null)"));
             compressionFailed(ctx);
             return false;
         }

lib/pal/PAL.cpp

@@ -369,19 +369,32 @@ namespace PAL_NS_BEGIN {
 	std::transform(uuidStr.begin(), uuidStr.end(), uuidStr.begin(), ::tolower);
         return uuidStr;
 #else
-        static std::once_flag flag;
-        std::call_once(flag, [](){
-            auto now = std::chrono::high_resolution_clock::now();
-            auto nanos = std::chrono::duration_cast<std::chrono::nanoseconds>(now.time_since_epoch()).count();
-            std::srand(static_cast<unsigned int>(std::time(0) ^ nanos));
-        });
+        // Use std::random_device -- a non-deterministic, CSPRNG-backed source on
+        // the platforms we target (glibc/bionic/libc++ draw from getrandom or
+        // /dev/urandom) -- instead of std::rand()/srand(time(0)), so the session
+        // and event identifiers built from it are not predictable. It is
+        // thread_local so the backing source is opened once per thread rather than
+        // on every call (generateUuidString is on the event logging hot path), and
+        // the 128 bits are drawn with four operator() calls instead of eleven
+        // (random_device::max() is guaranteed to span the full unsigned int range).
+        thread_local std::random_device rd;
 
         GUID_t uuid;
-        uuid.Data1 = (static_cast<uint16_t>(std::rand()) << 16) | static_cast<uint16_t>(std::rand());
-        uuid.Data2 = static_cast<uint16_t>(std::rand());
-        uuid.Data3 = static_cast<uint16_t>(std::rand());
-        for (size_t i = 0; i < sizeof(uuid.Data4); i++)
-            uuid.Data4[i] = static_cast<uint8_t>(std::rand());
+        const uint32_t r0 = rd();
+        const uint32_t r1 = rd();
+        const uint32_t r2 = rd();
+        const uint32_t r3 = rd();
+        uuid.Data1 = r0;
+        uuid.Data2 = static_cast<uint16_t>(r1);
+        uuid.Data3 = static_cast<uint16_t>(r1 >> 16);
+        uuid.Data4[0] = static_cast<uint8_t>(r2);
+        uuid.Data4[1] = static_cast<uint8_t>(r2 >> 8);
+        uuid.Data4[2] = static_cast<uint8_t>(r2 >> 16);
+        uuid.Data4[3] = static_cast<uint8_t>(r2 >> 24);
+        uuid.Data4[4] = static_cast<uint8_t>(r3);
+        uuid.Data4[5] = static_cast<uint8_t>(r3 >> 8);
+        uuid.Data4[6] = static_cast<uint8_t>(r3 >> 16);
+        uuid.Data4[7] = static_cast<uint8_t>(r3 >> 24);
 
         // TODO: [MG] - replace this sprintf by more robust GUID to string converter
         char buf[40] = { 0 };

lib/utils/ZlibUtils.cpp

@@ -48,7 +48,7 @@ namespace MAT_NS_BEGIN
         } while (ret == Z_OK);
         if (ret != Z_STREAM_END)
         {
-            LOG_WARN("Inflate failed, error=%u/%u (%s)", 2, ret, zs.msg);
+            LOG_WARN("Inflate failed, error=%d/%d (%s)", 2, ret, (zs.msg ? zs.msg : "(null)"));
             result = false;
         }
         inflateEnd(&zs);

tests/unittests/PalTests.cpp

@@ -9,6 +9,7 @@
 
 #include <cstdint>
 #include <string>
+#include <set>
 
 #ifdef HAVE_MAT_LOGGING
 #include "pal/PAL.hpp"
@@ -42,9 +43,14 @@ class PalTests : public Test {};
 
 TEST_F(PalTests, UuidGeneration)
 {
+    // Canonical UUID string length ("8-4-4-4-12") and the number of UUIDs
+    // generated for the uniqueness check below.
+    constexpr size_t UuidStringLength = 36;
+    constexpr size_t UuidBatchSize = 1000;
+
     std::string uuid0 = PAL::generateUuidString();
 
-    EXPECT_THAT(uuid0.length(), 36u);
+    EXPECT_THAT(uuid0.length(), UuidStringLength);
 
     std::string mask = uuid0;
     for (char& ch : mask) {
@@ -61,21 +67,31 @@ TEST_F(PalTests, UuidGeneration)
 
     std::string uuid1 = PAL::generateUuidString();
 
-    EXPECT_THAT(uuid1.length(), 36u);
+    EXPECT_THAT(uuid1.length(), UuidStringLength);
 
     size_t diff = 0;
-    for (size_t i = 0; i < 36; i++) {
+    for (size_t i = 0; i < UuidStringLength; i++) {
         diff += (uuid0[i] != uuid1[i]);
     }
     EXPECT_THAT(diff, Gt(20u));
+
+    // A batch of generated UUIDs must all be distinct (guards against a stuck
+    // or low-entropy generator).
+    std::set<std::string> uuids;
+    for (size_t i = 0; i < UuidBatchSize; i++) {
+        std::string u = PAL::generateUuidString();
+        EXPECT_THAT(u.length(), UuidStringLength);
+        uuids.insert(u);
+    }
+    EXPECT_THAT(uuids.size(), UuidBatchSize);
 }
 
 TEST_F(PalTests, PseudoRandomGenerator)
 {
     PAL::PseudoRandomGenerator prg;
 
-    size_t const NumQueries = 1000;
-    size_t const NumBuckets = 11;
+    constexpr size_t NumQueries = 1000;
+    constexpr size_t NumBuckets = 11;
     size_t buckets[NumBuckets] = {};
 
     for (size_t i = 0; i < NumQueries; i++) {