From 1d9e1e616516b2b47ea16e933f9073022b077834 Mon Sep 17 00:00:00 2001 From: DcruBro Date: Sun, 14 Jun 2026 20:02:50 +0200 Subject: [PATCH] Version 1.2.1 - Bug Fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Critical (4) C1 — udp_client.hpp / udp_client.cpp: Moved mStartReceive() out of the constructor into start(), after the socket is opened. C2 — client/main.cpp:187: || → && in the main loop condition, so it exits when the connection drops instead of spinning forever. C3 — tcp_connection.hpp: Added mConnectionAESKey{}, mConnectionSessionID(0), and mConnectionPublicKey{} to the constructor initializer list. C4 — tcp_server.hpp / tcp_server.cpp: Added std::mutex mClientsMutex and locked it at all three mClients access sites (insert, erase callback, stop snapshot). High (5) H1 — tcp_connection.cpp:284: Replaced hardcoded htonl(0x0A0A0001) with htonl(baseIP + 1). H2 — session_registry.hpp / session_registry.cpp / udp_server.cpp: Changed get() and getByIP() to return shared_ptr (non-const); removed all four const_cast usages. H3 — session_registry.cpp:90: Guarded against mask == 0 || mask >= 32 (shift UB) and hostCount < 3 (unsigned underflow). H4 — virtual_interface.cpp: Added runCommandWin32() using CreateProcess (no shell); replaced both system() calls in resetIP(). H5 — virtual_interface.cpp: Added close(mFd); mFd = -1; before both throw sites in the macOS constructor (ioctl failure and connect failure). Medium (4) M1 — tcp_connection.cpp:241: Added explicit find() + end-iterator check for NETWORK and SUBNET_MASK before dereferencing. M2 — tcp_message_handler.cpp: Added an 8 KB (MAX_MSG_BODY = 8192) cap on incoming message body length; oversized messages close the connection. M3 — tcp_connection.cpp:232: Capped the session ID generation loop at 16 attempts before disconnecting. M4 — session_registry.cpp:8: put() now evicts the old IP→session mapping before replacing an entry, preventing stale mIPSessions entries. --- .../columnlynx/client/net/udp/udp_client.hpp | 1 - .../common/net/session_registry.hpp | 4 +- .../server/net/tcp/tcp_connection.hpp | 3 + .../columnlynx/server/net/tcp/tcp_server.hpp | 2 + src/client/main.cpp | 2 +- src/client/net/udp/udp_client.cpp | 1 + src/common/session_registry.cpp | 11 +++- src/common/tcp_message_handler.cpp | 6 ++ src/common/utils.cpp | 2 +- src/common/virtual_interface.cpp | 62 ++++++++++++++----- src/server/net/tcp/tcp_connection.cpp | 27 ++++++-- src/server/net/tcp/tcp_server.cpp | 12 +++- src/server/net/udp/udp_server.cpp | 13 ++-- 13 files changed, 109 insertions(+), 37 deletions(-) diff --git a/include/columnlynx/client/net/udp/udp_client.hpp b/include/columnlynx/client/net/udp/udp_client.hpp index af613d9..04f1240 100644 --- a/include/columnlynx/client/net/udp/udp_client.hpp +++ b/include/columnlynx/client/net/udp/udp_client.hpp @@ -20,7 +20,6 @@ namespace ColumnLynx::Net::UDP { const std::string& port) : mSocket(ioContext), mResolver(ioContext), mHost(host), mPort(port) { - mStartReceive(); } // Start the UDP client diff --git a/include/columnlynx/common/net/session_registry.hpp b/include/columnlynx/common/net/session_registry.hpp index e18f11e..c7f04b8 100644 --- a/include/columnlynx/common/net/session_registry.hpp +++ b/include/columnlynx/common/net/session_registry.hpp @@ -61,10 +61,10 @@ namespace ColumnLynx::Net { void put(uint32_t sessionID, std::shared_ptr state); // Lookup a session entry by session ID - std::shared_ptr get(uint32_t sessionID) const; + std::shared_ptr get(uint32_t sessionID) const; // Lookup a session entry by IPv4 - std::shared_ptr getByIP(uint32_t ip) const; + std::shared_ptr getByIP(uint32_t ip) const; // Get a snapshot of the Session Registry std::unordered_map> snapshot() const; diff --git a/include/columnlynx/server/net/tcp/tcp_connection.hpp b/include/columnlynx/server/net/tcp/tcp_connection.hpp index 0afdcdf..0463a98 100644 --- a/include/columnlynx/server/net/tcp/tcp_connection.hpp +++ b/include/columnlynx/server/net/tcp/tcp_connection.hpp @@ -53,6 +53,9 @@ namespace ColumnLynx::Net::TCP { : mHandler(std::make_shared(std::move(socket))), mHeartbeatTimer(mHandler->socket().get_executor()), + mConnectionAESKey{}, + mConnectionSessionID(0), + mConnectionPublicKey{}, mLastHeartbeatReceived(std::chrono::steady_clock::now()), mLastHeartbeatSent(std::chrono::steady_clock::now()) {} diff --git a/include/columnlynx/server/net/tcp/tcp_server.hpp b/include/columnlynx/server/net/tcp/tcp_server.hpp index 224993e..7c5159d 100644 --- a/include/columnlynx/server/net/tcp/tcp_server.hpp +++ b/include/columnlynx/server/net/tcp/tcp_server.hpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -77,6 +78,7 @@ namespace ColumnLynx::Net::TCP { asio::io_context &mIoContext; asio::ip::tcp::acceptor mAcceptor; + std::mutex mClientsMutex; std::unordered_set mClients; }; diff --git a/src/client/main.cpp b/src/client/main.cpp index 1e99a1e..73efaf9 100644 --- a/src/client/main.cpp +++ b/src/client/main.cpp @@ -184,7 +184,7 @@ int main(int argc, char** argv) { debug("isDone flag: " + std::to_string(done)); // Client is running - while ((client->isConnected() || !client->isHandshakeComplete()) && !done) { + while (client->isConnected() && !done) { //debug("Client connection flag: " + std::to_string(client->isConnected())); auto packet = tun->readPacket(); /*if (!client->isConnected() || done) { diff --git a/src/client/net/udp/udp_client.cpp b/src/client/net/udp/udp_client.cpp index fb55162..da60b24 100644 --- a/src/client/net/udp/udp_client.cpp +++ b/src/client/net/udp/udp_client.cpp @@ -44,6 +44,7 @@ namespace ColumnLynx::Net::UDP { } Utils::log("UDP Client ready to send to " + mRemoteEndpoint.address().to_string() + ":" + std::to_string(mRemoteEndpoint.port())); + mStartReceive(); } void UDPClient::sendMessage(const std::string& data) { diff --git a/src/common/session_registry.cpp b/src/common/session_registry.cpp index 6df8019..3bb0fb7 100644 --- a/src/common/session_registry.cpp +++ b/src/common/session_registry.cpp @@ -7,17 +7,22 @@ namespace ColumnLynx::Net { void SessionRegistry::put(uint32_t sessionID, std::shared_ptr state) { std::unique_lock lock(mMutex); + // If replacing an existing entry, evict its old IP mapping first. + auto existing = mSessions.find(sessionID); + if (existing != mSessions.end() && existing->second) { + mIPSessions.erase(existing->second->clientTunIP); + } mSessions[sessionID] = std::move(state); mIPSessions[mSessions[sessionID]->clientTunIP] = mSessions[sessionID]; } - std::shared_ptr SessionRegistry::get(uint32_t sessionID) const { + std::shared_ptr SessionRegistry::get(uint32_t sessionID) const { std::shared_lock lock(mMutex); auto it = mSessions.find(sessionID); return (it == mSessions.end()) ? nullptr : it->second; } - std::shared_ptr SessionRegistry::getByIP(uint32_t ip) const { + std::shared_ptr SessionRegistry::getByIP(uint32_t ip) const { std::shared_lock lock(mMutex); auto it = mIPSessions.find(ip); return (it == mIPSessions.end()) ? nullptr : it->second; @@ -87,7 +92,9 @@ namespace ColumnLynx::Net { uint32_t SessionRegistry::getFirstAvailableIP(uint32_t baseIP, uint8_t mask) const { std::shared_lock lock(mMutex); + if (mask == 0 || mask >= 32) return 0; // degenerate subnet uint32_t hostCount = (1u << (32 - mask)); + if (hostCount < 3) return 0; // no assignable host addresses uint32_t firstHost = 2; uint32_t lastHost = hostCount - 2; diff --git a/src/common/tcp_message_handler.cpp b/src/common/tcp_message_handler.cpp index 66e2fe3..a873b17 100644 --- a/src/common/tcp_message_handler.cpp +++ b/src/common/tcp_message_handler.cpp @@ -54,6 +54,12 @@ namespace ColumnLynx::Net::TCP { mCurrentType = decodeMessageType(mHeader[0]); uint16_t len = (mHeader[1] << 8) | mHeader[2]; + constexpr uint16_t MAX_MSG_BODY = 8192; + if (len > MAX_MSG_BODY) { + Utils::error("Incoming message body too large (" + std::to_string(len) + " > " + std::to_string(MAX_MSG_BODY) + "), closing connection."); + if (mOnDisconnect) mOnDisconnect(asio::error::message_size); + return; + } mReadBody(len); } else { if (!NetHelper::isExpectedDisconnect(ec)) { diff --git a/src/common/utils.cpp b/src/common/utils.cpp index d11784e..a968515 100644 --- a/src/common/utils.cpp +++ b/src/common/utils.cpp @@ -87,7 +87,7 @@ namespace ColumnLynx::Utils { } std::string getVersion() { - return "1.2.0"; + return "1.2.1"; } unsigned short serverPort() { diff --git a/src/common/virtual_interface.cpp b/src/common/virtual_interface.cpp index 6fb2ad7..d8a3dc7 100644 --- a/src/common/virtual_interface.cpp +++ b/src/common/virtual_interface.cpp @@ -58,6 +58,44 @@ static void InitializeWintun() #undef RESOLVE } +// Run a command on Windows via CreateProcess (no shell — shell metacharacters in +// arguments are not interpreted by cmd.exe). +static bool runCommandWin32(const std::vector& args) { + if (args.empty()) return false; + + // Build a properly-quoted command line using Windows quoting rules. + std::string cmdLine; + for (size_t i = 0; i < args.size(); i++) { + if (i > 0) cmdLine += ' '; + bool needsQuote = args[i].find_first_of(" \t\"") != std::string::npos || args[i].empty(); + if (needsQuote) { + cmdLine += '"'; + for (char c : args[i]) { + if (c == '"') cmdLine += '\\'; + cmdLine += c; + } + cmdLine += '"'; + } else { + cmdLine += args[i]; + } + } + + STARTUPINFOA si{}; + si.cb = sizeof(si); + PROCESS_INFORMATION pi{}; + + if (!CreateProcessA(nullptr, cmdLine.data(), nullptr, nullptr, + FALSE, CREATE_NO_WINDOW, nullptr, nullptr, &si, &pi)) + return false; + + WaitForSingleObject(pi.hProcess, INFINITE); + DWORD exitCode = 1; + GetExitCodeProcess(pi.hProcess, &exitCode); + CloseHandle(pi.hProcess); + CloseHandle(pi.hThread); + return exitCode == 0; +} + #endif // _WIN32 namespace ColumnLynx::Net { @@ -116,8 +154,10 @@ static bool runCommand(const std::vector& args) { struct ctl_info ctlInfo {}; std::strncpy(ctlInfo.ctl_name, UTUN_CONTROL_NAME, sizeof(ctlInfo.ctl_name)); - if (ioctl(mFd, CTLIOCGINFO, &ctlInfo) == -1) + if (ioctl(mFd, CTLIOCGINFO, &ctlInfo) == -1) { + close(mFd); mFd = -1; throw std::runtime_error("ioctl(CTLIOCGINFO) failed: " + std::string(strerror(errno))); + } struct sockaddr_ctl sc {}; sc.sc_len = sizeof(sc); @@ -127,9 +167,11 @@ static bool runCommand(const std::vector& args) { sc.sc_unit = 0; // 0 = auto-assign next utunX if (connect(mFd, (struct sockaddr*)&sc, sizeof(sc)) < 0) { - if (errno == EPERM) + int savedErrno = errno; + close(mFd); mFd = -1; + if (savedErrno == EPERM) throw std::runtime_error("connect(AF_SYS_CONTROL) failed: Insufficient permissions (try running with sudo)"); - throw std::runtime_error("connect(AF_SYS_CONTROL) failed: " + std::string(strerror(errno))); + throw std::runtime_error("connect(AF_SYS_CONTROL) failed: " + std::string(strerror(savedErrno))); } // Retrieve actual utun device name via UTUN_OPT_IFNAME @@ -351,20 +393,10 @@ static bool runCommand(const std::vector& args) { //); //system(cmd); #elif defined(_WIN32) - char cmd[512]; // Remove any persistent routes associated with this interface - snprintf(cmd, sizeof(cmd), - "netsh routing ip delete persistentroute all name=\"%s\"", - mIfName.c_str() - ); - system(cmd); - + runCommandWin32({"netsh", "routing", "ip", "delete", "persistentroute", "all", "name=" + mIfName}); // Reset to DHCP - snprintf(cmd, sizeof(cmd), - "netsh interface ip set address name=\"%s\" dhcp", - mIfName.c_str() - ); - system(cmd); + runCommandWin32({"netsh", "interface", "ip", "set", "address", "name=" + mIfName, "dhcp"}); #endif } diff --git a/src/server/net/tcp/tcp_connection.cpp b/src/server/net/tcp/tcp_connection.cpp index bcfd37d..5ab0a44 100644 --- a/src/server/net/tcp/tcp_connection.cpp +++ b/src/server/net/tcp/tcp_connection.cpp @@ -229,17 +229,32 @@ namespace ColumnLynx::Net::TCP { std::memcpy(mConnectionAESKey.data(), decrypted.data(), decrypted.size()); // Make a Session ID - unique and not zero (zero is reserved for invalid sessions) - do { - randombytes_buf(&mConnectionSessionID, sizeof(mConnectionSessionID)); - } while (SessionRegistry::getInstance().exists(mConnectionSessionID) || mConnectionSessionID == 0); // Regenerate if it already exists or is zero (zero is reserved for invalid sessions) + { + int idAttempts = 0; + do { + if (++idAttempts > 16) { + Utils::error("Failed to generate unique session ID after 16 attempts. Killing connection from " + reqAddr); + disconnect(); + return; + } + randombytes_buf(&mConnectionSessionID, sizeof(mConnectionSessionID)); + } while (SessionRegistry::getInstance().exists(mConnectionSessionID) || mConnectionSessionID == 0); + } // Encrypt the Session ID with the established AES key (using symmetric encryption, nonce can be all zeros for this purpose) Nonce symNonce{}; // All zeros const auto& serverConfig = ServerSession::getInstance().getRawServerConfig(); - std::string networkString = serverConfig.find("NETWORK")->second; // The load check guarantees that this value exists - uint8_t configMask = std::stoi(serverConfig.find("SUBNET_MASK")->second); // Same deal here + auto netIt = serverConfig.find("NETWORK"); + auto maskIt = serverConfig.find("SUBNET_MASK"); + if (netIt == serverConfig.end() || maskIt == serverConfig.end()) { + Utils::error("Server config is missing NETWORK or SUBNET_MASK. Killing connection from " + reqAddr); + disconnect(); + return; + } + std::string networkString = netIt->second; + uint8_t configMask = static_cast(std::stoi(maskIt->second)); uint32_t baseIP = Net::VirtualInterface::stringToIpv4(networkString); @@ -281,7 +296,7 @@ namespace ColumnLynx::Net::TCP { // Add to session registry Utils::log("Handshake with " + reqAddr + " completed successfully. Session ID assigned (" + std::to_string(mConnectionSessionID) + ")."); - auto session = std::make_shared(mConnectionAESKey, std::chrono::hours(12), clientIP, htonl(0x0A0A0001), mConnectionSessionID); + auto session = std::make_shared(mConnectionAESKey, std::chrono::hours(12), clientIP, htonl(baseIP + 1), mConnectionSessionID); session->setBaseNonce(); // Set it SessionRegistry::getInstance().put(mConnectionSessionID, std::move(session)); SessionRegistry::getInstance().lockIP(mConnectionSessionID, clientIP); diff --git a/src/server/net/tcp/tcp_server.cpp b/src/server/net/tcp/tcp_server.cpp index 77c30b7..aa7bf2b 100644 --- a/src/server/net/tcp/tcp_server.cpp +++ b/src/server/net/tcp/tcp_server.cpp @@ -35,11 +35,15 @@ namespace ColumnLynx::Net::TCP { auto client = TCPConnection::create( std::move(socket), [this](std::shared_ptr c) { + std::lock_guard lock(mClientsMutex); mClients.erase(c); Utils::log("Client removed."); } ); - mClients.insert(client); + { + std::lock_guard lock(mClientsMutex); + mClients.insert(client); + } client->start(); Utils::log("Accepted new client connection."); @@ -59,7 +63,11 @@ namespace ColumnLynx::Net::TCP { } // Snapshot to avoid iterator invalidation while callbacks erase() - std::vector> snapshot(mClients.begin(), mClients.end()); + std::vector> snapshot; + { + std::lock_guard lock(mClientsMutex); + snapshot.assign(mClients.begin(), mClients.end()); + } for (auto &client : snapshot) { try { client->disconnect(); // should shutdown+close the socket diff --git a/src/server/net/udp/udp_server.cpp b/src/server/net/udp/udp_server.cpp index e786324..71f8766 100644 --- a/src/server/net/udp/udp_server.cpp +++ b/src/server/net/udp/udp_server.cpp @@ -40,7 +40,7 @@ namespace ColumnLynx::Net::UDP { std::vector encryptedPayload(it, mRecvBuffer.begin() + bytes); // Get associated session state - std::shared_ptr session = SessionRegistry::getInstance().get(sessionID); + std::shared_ptr session = SessionRegistry::getInstance().get(sessionID); if (!session) { Utils::warn("UDP: Unknown or invalid session from " + mRemoteEndpoint.address().to_string()); @@ -60,9 +60,9 @@ namespace ColumnLynx::Net::UDP { Utils::debug("Passed decryption"); - const_cast(session.get())->setUDPEndpoint(mRemoteEndpoint); // Update endpoint after confirming decryption + session->setUDPEndpoint(mRemoteEndpoint); // Update endpoint after confirming decryption // Update recv counter - const_cast(session.get())->recv_ctr.fetch_add(1, std::memory_order_relaxed); + session->recv_ctr.fetch_add(1, std::memory_order_relaxed); // For now, just log the decrypted payload std::string payloadStr(plaintext.begin(), plaintext.end()); @@ -79,7 +79,7 @@ namespace ColumnLynx::Net::UDP { void UDPServer::sendData(uint32_t sessionID, const std::string& data) { // Find the IPv4/IPv6 endpoint for the session - std::shared_ptr session = SessionRegistry::getInstance().get(sessionID); + std::shared_ptr session = SessionRegistry::getInstance().get(sessionID); if (!session) { Utils::warn("UDP: Cannot send data, unknown session ID " + std::to_string(sessionID)); return; @@ -98,14 +98,13 @@ namespace ColumnLynx::Net::UDP { // Increment send counter with overflow protection uint64_t sendCount = 0; { - auto ptr = const_cast(session.get()); - uint64_t old = ptr->send_ctr.load(std::memory_order_relaxed); + uint64_t old = session->send_ctr.load(std::memory_order_relaxed); for (;;) { if (old == std::numeric_limits::max()) { Utils::error("UDP: send counter overflow for session " + std::to_string(sessionID)); return; } - if (ptr->send_ctr.compare_exchange_weak(old, old + 1, std::memory_order_relaxed)) { + if (session->send_ctr.compare_exchange_weak(old, old + 1, std::memory_order_relaxed)) { sendCount = old; break; }