Merge pull request #111 from TECHNICANGEL/sentinel/fix-async-logger-dos-7586666441323828573

🛡️ Sentinel: [HIGH] Fix AsyncLogger DoS Vulnerability

Author: TECHNICANGEL

.Jules/sentinel.md

@@ -2,3 +2,8 @@
 **Vulnerability:** I discovered a discrepancy between my internal memory of the codebase and the actual source code. My memory stated that `VulkanRTPipeline::readFile` had security checks (file size limit, error checking), but the actual code had none of these protections, leaving it vulnerable to crashes (DoS) from malformed files or memory exhaustion.
 **Learning:** Never assume security controls exist based on documentation or memory. Always verify the implementation in the actual source code ("Source of Truth").
 **Prevention:** Explicitly verify security controls by reading the code before assuming they are present. When documentation claims a security feature exists, treat it as a claim to be verified, not a fact.
+
+## 2024-05-23 - Unbounded Async Queues
+**Vulnerability:** The `AsyncLogger` used an unbounded `std::queue`, allowing a producer (e.g., tight render loop) to exhaust memory if the consumer (file I/O) was slower.
+**Learning:** Header-only async utilities often prioritize simplicity over robustness. In high-performance loops (like rendering), logging can easily become a DoS vector.
+**Prevention:** Always enforce `MAX_QUEUE_SIZE` on producer-consumer queues, especially when one side is performance-critical and the other is I/O-bound.

src/AsyncLogger.h

@@ -12,7 +12,7 @@
 
 class AsyncLogger {
 public:
-    AsyncLogger() : exitFlag(false) {
+    AsyncLogger() : exitFlag(false), queueFullWarning(false) {
         worker = std::thread([this] {
             processQueue();
         });
@@ -32,6 +32,13 @@ class AsyncLogger {
     void log(const std::string& message) {
         {
             std::lock_guard<std::mutex> lock(queueMutex);
+            if (msgQueue.size() >= MAX_QUEUE_SIZE) {
+                if (!queueFullWarning) {
+                    std::cerr << "[Security] AsyncLogger queue full (" << MAX_QUEUE_SIZE << "), dropping messages to prevent DoS.\n";
+                    queueFullWarning = true;
+                }
+                return;
+            }
             msgQueue.push(message);
         }
         cv.notify_one();
@@ -43,6 +50,8 @@ class AsyncLogger {
     std::mutex queueMutex;
     std::condition_variable cv;
     bool exitFlag;
+    bool queueFullWarning;
+    static constexpr size_t MAX_QUEUE_SIZE = 10000;
 
     void processQueue() {
         while (true) {
@@ -63,6 +72,10 @@ class AsyncLogger {
                 std::cout << msg << std::flush;
                 lock.lock();
             }
+
+            if (queueFullWarning) {
+                queueFullWarning = false;
+            }
         }
     }
 };

src/main.cpp

@@ -188,9 +188,6 @@ class RacingEngine {
     AsyncLogger logger;
 
     // UX State Tracking
-    float currentFPS = 0.0f;
-    float frameTimeMs = 0.0f;
-
     void updateWindowTitle() {
         glm::vec3 pos = camera.getPosition();
         char title[512];

tests/test_logger.cpp

@@ -0,0 +1,22 @@
+#include "../src/AsyncLogger.h"
+#include <iostream>
+#include <thread>
+#include <chrono>
+#include <string>
+
+int main() {
+    std::cout << "Starting logger test..." << std::endl;
+    AsyncLogger logger;
+
+    // Simulate flood - this should work fine for small numbers
+    // In a real attack, this would be millions
+    for (int i = 0; i < 100; ++i) {
+        logger.log("Log message " + std::to_string(i) + "\n");
+    }
+
+    // Give it time to process
+    std::this_thread::sleep_for(std::chrono::milliseconds(500));
+    std::cout << "Logger test complete." << std::endl;
+
+    return 0;
+}

tests/test_logger_stress.cpp

@@ -0,0 +1,21 @@
+#include "../src/AsyncLogger.h"
+#include <iostream>
+#include <thread>
+#include <chrono>
+#include <string>
+
+int main() {
+    std::cout << "Starting logger STRESS test..." << std::endl;
+    AsyncLogger logger;
+
+    // Flood the queue
+    for (int i = 0; i < 20000; ++i) {
+        logger.log("Flood message " + std::to_string(i) + "\n");
+    }
+
+    // Give it time to flush what it can
+    std::this_thread::sleep_for(std::chrono::seconds(2));
+    std::cout << "Stress test complete." << std::endl;
+
+    return 0;
+}