From b7008427f9be5e2a3e25f97dd2ee99afa96b395b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 25 Dec 2025 08:34:14 +0000 Subject: [PATCH] Address code review feedback - fix race condition, add size_t validation, improve tests Co-authored-by: johndoe6345789 <224850594+johndoe6345789@users.noreply.github.com> --- dbal/cpp/src/daemon/server.cpp | 14 +++++++++++--- .../tests/security/http_server_security_test.cpp | 13 +++++++++---- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/dbal/cpp/src/daemon/server.cpp b/dbal/cpp/src/daemon/server.cpp index deee4cbbe..1758c3594 100644 --- a/dbal/cpp/src/daemon/server.cpp +++ b/dbal/cpp/src/daemon/server.cpp @@ -275,14 +275,15 @@ private: } // Check connection limit to prevent thread exhaustion DoS - if (active_connections_.load() >= MAX_CONCURRENT_CONNECTIONS) { + // Use atomic fetch_add to avoid race condition + size_t prev_count = active_connections_.fetch_add(1); + if (prev_count >= MAX_CONCURRENT_CONNECTIONS) { std::cerr << "Connection limit reached, rejecting connection" << std::endl; + active_connections_--; CLOSE_SOCKET(client_fd); continue; } - active_connections_++; - // Handle connection in a new thread std::thread(&Server::handleConnection, this, client_fd).detach(); } @@ -486,6 +487,13 @@ private: error_response.body = R"({"error":"Content-Length too large"})"; return false; } + // Validate fits in size_t (platform dependent) + if (cl > std::numeric_limits::max()) { + error_response.status_code = 413; + error_response.status_text = "Request Entity Too Large"; + error_response.body = R"({"error":"Content-Length exceeds platform limit"})"; + return false; + } content_length = static_cast(cl); } catch (...) { error_response.status_code = 400; diff --git a/dbal/cpp/tests/security/http_server_security_test.cpp b/dbal/cpp/tests/security/http_server_security_test.cpp index 7e924e0ff..8f4d14588 100644 --- a/dbal/cpp/tests/security/http_server_security_test.cpp +++ b/dbal/cpp/tests/security/http_server_security_test.cpp @@ -225,7 +225,8 @@ public: std::string part1 = "GET /api/status HTTP/1.1\r\n"; send(sock, part1.c_str(), part1.length(), 0); - std::this_thread::sleep_for(std::chrono::seconds(5)); + // Wait 2 seconds (reduced for faster tests) + std::this_thread::sleep_for(std::chrono::seconds(2)); std::string part2 = "Host: localhost\r\n"; int result = send(sock, part2.c_str(), part2.length(), 0); @@ -293,10 +294,14 @@ public: if (bytes > 0) { buffer[bytes] = '\0'; std::string response(buffer); - // Should not expose filesystem + // Should get 400 Bad Request for null byte + bool rejected = response.find("400") != std::string::npos || + response.find("Bad Request") != std::string::npos; + // Also verify no sensitive content exposed bool safe = response.find("passwd") == std::string::npos; - std::cout << " " << (safe ? "PASS: Safe" : "FAIL: Vulnerable") << std::endl; - return safe; + bool pass = rejected && safe; + std::cout << " " << (pass ? "PASS: Null byte rejected" : "FAIL: Vulnerable") << std::endl; + return pass; } return false;