From ccdfcff64e533e35def85cacfa20e14fc3570630 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 20 Feb 2026 19:32:30 +0000 Subject: [PATCH] Fix Unbounded Queue Vulnerability in AsyncLogger - Add `MAX_QUEUE_SIZE` (10000) to `AsyncLogger` to prevent memory exhaustion (DoS). - Implement message dropping mechanism when queue is full. - Add thread-safe warning mechanism (`queueFullWarning`) to log overflow events to `stderr` without spamming. - Update `.Jules/sentinel.md` with DoS/Unbounded Queue learning. - Fix duplicate member declaration in `src/main.cpp` to fix compilation. Co-authored-by: TECHNICANGEL <197574689+TECHNICANGEL@users.noreply.github.com> --- .Jules/sentinel.md | 5 +++++ src/AsyncLogger.h | 16 +++++++++++++++- src/main.cpp | 3 +-- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/.Jules/sentinel.md b/.Jules/sentinel.md index 1c1ddfe..3ae5cc5 100644 --- a/.Jules/sentinel.md +++ b/.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 - Denial of Service: Unbounded Queues +**Vulnerability:** The `AsyncLogger` class used an unbounded `std::queue` to buffer log messages. If the producer (main thread) generated logs faster than the consumer (worker thread) could write to stdout, the queue would grow indefinitely, leading to memory exhaustion and application crash (DoS). +**Learning:** Asynchronous operations must always have resource limits. "Infinite" buffering is a security anti-pattern. +**Prevention:** Implement bounded queues (e.g., ring buffers or size checks) for all producer-consumer patterns. Drop data or block the producer when limits are reached, depending on the criticality of the data. diff --git a/src/AsyncLogger.h b/src/AsyncLogger.h index 5c2ccdc..d705692 100644 --- a/src/AsyncLogger.h +++ b/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 lock(queueMutex); + if (msgQueue.size() >= MAX_QUEUE_SIZE) { + if (!queueFullWarning) { + std::cerr << "[Security] AsyncLogger queue full (" << MAX_QUEUE_SIZE << "), dropping messages!\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,11 @@ class AsyncLogger { std::cout << msg << std::flush; lock.lock(); } + + // Queue is empty here. Reset warning flag if it was set. + if (queueFullWarning) { + queueFullWarning = false; + } } } }; diff --git a/src/main.cpp b/src/main.cpp index 3503741..17afb2f 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -188,8 +188,7 @@ class RacingEngine { AsyncLogger logger; // UX State Tracking - float currentFPS = 0.0f; - float frameTimeMs = 0.0f; + // Note: currentFPS and frameTimeMs are already defined above void updateWindowTitle() { glm::vec3 pos = camera.getPosition();