From 2916351ec481efdcb9311d4e5a41f85f5fdc6abc Mon Sep 17 00:00:00 2001 From: Oden Eriksson Date: Mon, 22 Sep 2025 08:02:05 +0200 Subject: [PATCH 1/9] =?UTF-8?q?Use=20cryptographically=20secure=20randomne?= =?UTF-8?q?ss=20for=20the=2016=E2=80=91byte=20WebSocket=20Sec-WebSocket-Ke?= =?UTF-8?q?y.=20Prefer=20gnutls=5Frnd=20when=20GNUTLS=20is=20enabled,=20ot?= =?UTF-8?q?herwise=20read=20from=20/dev/urandom=20and=20only=20fall=20back?= =?UTF-8?q?=20to=20rand()=20as=20a=20last=20resort.=20This=20avoids=20pred?= =?UTF-8?q?ictable=20keys=20and=20aligns=20with=20RFC=206455=20requirement?= =?UTF-8?q?s.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix the previous change to use std::array correctly: - Use rnd.data() instead of casting the whole array - Use rnd.size() for length Also add necessary includes. This ensures C++11 builds and avoids invalid reinterpret_cast. These changes were discovered by using ChatGPT and I take no credit for the contributions. It builds and works under Red Hat Fedora 42. --- src/http/httpclientconnection.cpp | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/http/httpclientconnection.cpp b/src/http/httpclientconnection.cpp index 9303c6b..2a437ff 100644 --- a/src/http/httpclientconnection.cpp +++ b/src/http/httpclientconnection.cpp @@ -13,6 +13,12 @@ #include "httpclienttask.h" #include "http_common.h" #include "sha1.h" +#include +#include +#include +#ifdef USE_GNUTLS +#include +#endif HttpClientConnection::HttpClientConnection(const std::string &label, HttpClientTask *task, @@ -342,10 +348,21 @@ void HttpClientConnection::ws_get(const std::string &url) { static char dst[] { "01234567890123456789====258EAFA5-E914-47DA-95CA-C5AB0DC85B11" }; - // 16 byte random data, convert to 24 base64 chars: - int32_t rnd[4] = { rand(), rand(), rand(), rand() }; - const unsigned char *src = reinterpret_cast(rnd); - base64_encode(src, sizeof(rnd), dst); + // 16-byte random data, convert to 24 base64 chars (RFC 6455) + std::array rnd{}; +#ifdef USE_GNUTLS + gnutls_rnd(GNUTLS_RND_NONCE, rnd.data(), rnd.size()); +#else + int fd = open("/dev/urandom", O_RDONLY); + if (fd >= 0) { + ssize_t n = read(fd, rnd.data(), rnd.size()); + (void)n; + close(fd); + } else { + for (size_t i = 0; i < rnd.size(); ++i) rnd[i] = static_cast(rand()); + } +#endif + base64_encode(rnd.data(), rnd.size(), dst); current_uri = url; current_request = "GET " + proto_hostname + From be86b54a38a7a2e76b5fdc4eaee064ced1eeb30c Mon Sep 17 00:00:00 2001 From: Oden Eriksson Date: Mon, 22 Sep 2025 08:02:43 +0200 Subject: [PATCH 2/9] Remove unconditional GNUTLS_VERIFY_ALLOW_BROKEN and gate it behind an ALLOW_BROKEN_TLS macro (disabled by default). This avoids accepting broken certificate chains in normal builds. These changes were discovered by using ChatGPT and I take no credit for the contributions. It builds and works under Red Hat Fedora 42. --- src/framework/socketconnection.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/framework/socketconnection.cpp b/src/framework/socketconnection.cpp index e135244..34965b1 100644 --- a/src/framework/socketconnection.cpp +++ b/src/framework/socketconnection.cpp @@ -247,8 +247,12 @@ init_tls_client(gnutls_certificate_credentials_t &x509_cred, bool verify_cert) { gnutls_session_set_verify_cert(session, peer_ip.c_str(), 0); } +#ifndef ALLOW_BROKEN_TLS + // Do not weaken verification by default +#else gnutls_certificate_set_verify_flags (x509_cred, GNUTLS_VERIFY_ALLOW_BROKEN); +#endif gnutls_handshake_set_timeout(session, GNUTLS_DEFAULT_HANDSHAKE_TIMEOUT); From 0dbb7d37b649b3114917838e9c3528199864bd9b Mon Sep 17 00:00:00 2001 From: Oden Eriksson Date: Mon, 22 Sep 2025 08:03:03 +0200 Subject: [PATCH 3/9] Ensure file descriptors are not inherited by child processes: - Set FD_CLOEXEC on created sockets. - Apply O_CLOEXEC on both ends of socketpair. This reduces risk of fd leaks across exec. These changes were discovered by using ChatGPT and I take no credit for the contributions. It builds and works under Red Hat Fedora 42. --- src/framework/socket.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/framework/socket.cpp b/src/framework/socket.cpp index b2c90a7..9f11b12 100644 --- a/src/framework/socket.cpp +++ b/src/framework/socket.cpp @@ -23,6 +23,14 @@ #endif #include + +static inline void set_cloexec(int fd) { + if (fd >= 0) { + int flags = fcntl(fd, F_GETFD); + if (flags != -1) fcntl(fd, F_SETFD, flags | FD_CLOEXEC); + } +} + #include #include @@ -56,8 +64,10 @@ Socket::Socket(const std::string &label, Task *owner, } else { _socket = pair_sd[0]; unix_domain_peer = pair_sd[1]; + set_cloexec(_socket); + set_cloexec(unix_domain_peer); fcntl(pair_sd[0], F_SETFL, O_NONBLOCK|O_CLOEXEC); - fcntl(pair_sd[1], F_SETFL, O_NONBLOCK); + fcntl(pair_sd[1], F_SETFL, O_NONBLOCK|O_CLOEXEC); } return; } From 80b6c52db33d8a0fb8f719496eb5597a89d450e7 Mon Sep 17 00:00:00 2001 From: Oden Eriksson Date: Mon, 22 Sep 2025 08:03:27 +0200 Subject: [PATCH 4/9] Avoid leaking file descriptors to child processes. Close all fds >= 3 in the child branch prior to execvp, using /proc/self/fd on Linux or a getrlimit(RLIMIT_NOFILE) fallback elsewhere. These changes were discovered by using ChatGPT and I take no credit for the contributions. It builds and works under Red Hat Fedora 42. --- src/framework/eventloop.cpp | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/src/framework/eventloop.cpp b/src/framework/eventloop.cpp index 6bf20e5..fe57e5e 100644 --- a/src/framework/eventloop.cpp +++ b/src/framework/eventloop.cpp @@ -79,9 +79,30 @@ int EventLoop::externalCommand(Task *owner, const char *const argv[]) { return -1; } if (!chld) { - // TODO: Close all file descriptors in child! + // Close all file descriptors >= 3 in the child before exec + #if defined(__linux__) && !defined(_WIN32) + // Best effort: use /proc/self/fd + { + DIR *d = opendir("/proc/self/fd"); + if (d) { + int dirfd_no = dirfd(d); + struct dirent *de; + while ((de = readdir(d))) { + int fd = atoi(de->d_name); + if (fd > 2 && fd != dirfd_no) close(fd); + } + closedir(d); + } + } + #else + // Fallback: close up to RLIMIT_NOFILE + struct rlimit rl; + if (getrlimit(RLIMIT_NOFILE, &rl) == 0) { + for (int fd = 3; fd < (int)rl.rlim_cur; ++fd) close(fd); + } + #endif int ret = execvp( argv[0], const_cast(argv) ); - exit(ret); + _exit(ret); } pidOwner[chld] = owner; return chld; From aeee9beeb0959fe78a5e6c3857adc6337198da57 Mon Sep 17 00:00:00 2001 From: Oden Eriksson Date: Mon, 22 Sep 2025 08:03:55 +0200 Subject: [PATCH 5/9] Fix build error by including for DIR/opendir/readdir, for getrlimit, and for atoi. These changes were discovered by using ChatGPT and I take no credit for the contributions. It builds and works under Red Hat Fedora 42. --- src/framework/eventloop.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/framework/eventloop.cpp b/src/framework/eventloop.cpp index fe57e5e..4273048 100644 --- a/src/framework/eventloop.cpp +++ b/src/framework/eventloop.cpp @@ -31,6 +31,9 @@ void EventLoop::signalHandler(int signum) { #include "socketreceiver.h" #include "workerprocess.h" +#include +#include +#include #ifdef USE_THREADS thread_local From 0a524435e1923a2cb9a448bd0f761efb521ca179 Mon Sep 17 00:00:00 2001 From: Oden Eriksson Date: Mon, 22 Sep 2025 08:04:19 +0200 Subject: [PATCH 6/9] Protect external_cmd() by: - Providing NO_EXTERNAL_CMD build-time switch to disable execution. - Rejecting dangerous shell metacharacters before calling popen. Note: For highest safety, prefer execvp with argv instead of shell. These changes were discovered by using ChatGPT and I take no credit for the contributions. It builds and works under Red Hat Fedora 42. --- src/measurement/defs.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/measurement/defs.cpp b/src/measurement/defs.cpp index fdbe36d..0db99da 100644 --- a/src/measurement/defs.cpp +++ b/src/measurement/defs.cpp @@ -81,7 +81,20 @@ namespace { #ifdef _WIN32 std::shared_ptr pipe(_popen(cmd, "r"), _pclose); #else + #ifdef NO_EXTERNAL_CMD + (void)cmd; + return std::string(); +#else + // Basic guard: reject characters that imply shell metacharacters + for (const char *p = cmd; *p; ++p) { + switch (*p) { + case ';': case '|': case '&': case '`': case '$': + case '>': case '<': case '\n': case '\r': + return std::string(); + } + } std::shared_ptr pipe(popen(cmd, "r"), pclose); +#endif #endif if (pipe) { while (!feof(pipe.get())) From ea7680de72133dd44254362b365bcc62d8232795 Mon Sep 17 00:00:00 2001 From: Oden Eriksson Date: Mon, 22 Sep 2025 08:04:39 +0200 Subject: [PATCH 7/9] Fix multiple robustness issues in json11: - Include to define uint8_t. - Add bounds checks before accessing value[i+1] and value[i+2]. - Cast to unsigned in snprintf when emitting \u escapes. These changes were discovered by using ChatGPT and I take no credit for the contributions. It builds and works under Red Hat Fedora 42. --- src/json11/json11.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/json11/json11.cpp b/src/json11/json11.cpp index 56ae4e8..ada6654 100644 --- a/src/json11/json11.cpp +++ b/src/json11/json11.cpp @@ -22,6 +22,7 @@ // Modified by Göran Andersson #include "json11.hpp" +#include #include #include #include @@ -98,13 +99,15 @@ static void dump(const string &value, string &out) { out += "\\t"; } else if (static_cast(ch) <= 0x1f) { char buf[8]; - snprintf(buf, sizeof buf, "\\u%04x", ch); + snprintf(buf, sizeof buf, "\\u%04x", static_cast(static_cast(ch))); out += buf; - } else if (static_cast(ch) == 0xe2 && static_cast(value[i+1]) == 0x80 + } else if (i + 2 < value.size() + && static_cast(ch) == 0xe2 && static_cast(value[i+1]) == 0x80 && static_cast(value[i+2]) == 0xa8) { out += "\\u2028"; i += 2; - } else if (static_cast(ch) == 0xe2 && static_cast(value[i+1]) == 0x80 + } else if (i + 2 < value.size() + && static_cast(ch) == 0xe2 && static_cast(value[i+1]) == 0x80 && static_cast(value[i+2]) == 0xa9) { out += "\\u2029"; i += 2; From 23094d4b499f02bfd49c24d6a67a90cff7cecbef Mon Sep 17 00:00:00 2001 From: Oden Eriksson Date: Mon, 22 Sep 2025 08:04:59 +0200 Subject: [PATCH 8/9] Generate upload buffer content with cryptographically strong bytes. - Prefer gnutls_rnd when GNUTLS is enabled - Otherwise read from /dev/urandom with a final rand() fallback Add required headers so this compiles under C++11. These changes were discovered by using ChatGPT and I take no credit for the contributions. It builds and works under Red Hat Fedora 42. --- src/measurement/wsuploadtask.cpp | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/measurement/wsuploadtask.cpp b/src/measurement/wsuploadtask.cpp index b093e62..0b74e3f 100644 --- a/src/measurement/wsuploadtask.cpp +++ b/src/measurement/wsuploadtask.cpp @@ -3,6 +3,12 @@ #include "wsuploadtask.h" #include +#include +#include +#ifdef USE_GNUTLS +#include +#endif +#include WsUploadTask::WsUploadTask(const std::string &ticket, const HttpHost &server, unsigned int no_conn, unsigned int max_conn, @@ -27,7 +33,17 @@ namespace { std::vector buf; // Fill the buffer with dummy data that cannot cause data compression while (len) { - buf.push_back(static_cast(rand())); +#ifdef USE_GNUTLS + unsigned char x; + gnutls_rnd(GNUTLS_RND_NONCE, &x, 1); + buf.push_back(static_cast(x)); +#else + int fd = open("/dev/urandom", O_RDONLY); + unsigned char x; + if (fd >= 0) { read(fd, &x, 1); close(fd); } + else { x = static_cast(rand()); } + buf.push_back(static_cast(x)); +#endif --len; } return buf; From 7d8fb32076e45a68b85ee205af5056eb094b7f8f Mon Sep 17 00:00:00 2001 From: Oden Eriksson Date: Mon, 22 Sep 2025 08:05:38 +0200 Subject: [PATCH 9/9] Replace hardcoded /usr/local include and lib paths with pkg-config queries so Fedora (and other distros) link correctly out of the box. These changes were discovered by using ChatGPT and I take no credit for the contributions. It builds and works under Red Hat Fedora 42. --- src/framework/mk.inc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/framework/mk.inc b/src/framework/mk.inc index 2819fd7..7637323 100644 --- a/src/framework/mk.inc +++ b/src/framework/mk.inc @@ -56,8 +56,8 @@ CXXFLAGS += --pedantic -Wextra endif ifeq ($(GNUTLS),1) -CXXFLAGS += -DUSE_GNUTLS -I/usr/local/include -LIBS += -L/usr/local/lib -lgnutls +CXXFLAGS += -DUSE_GNUTLS $(shell pkg-config --cflags gnutls) +LIBS += $(shell pkg-config --libs gnutls) endif ifeq ($(THREADS),1) CXXFLAGS += -DUSE_THREADS