Version 1.2.1 - Bug Fixes

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<SessionState> (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.
This commit is contained in:
2026-06-14 20:02:50 +02:00
parent 05febee79e
commit 1d9e1e6165
13 changed files with 109 additions and 37 deletions

View File

@@ -20,7 +20,6 @@ namespace ColumnLynx::Net::UDP {
const std::string& port) const std::string& port)
: mSocket(ioContext), mResolver(ioContext), mHost(host), mPort(port) : mSocket(ioContext), mResolver(ioContext), mHost(host), mPort(port)
{ {
mStartReceive();
} }
// Start the UDP client // Start the UDP client

View File

@@ -61,10 +61,10 @@ namespace ColumnLynx::Net {
void put(uint32_t sessionID, std::shared_ptr<SessionState> state); void put(uint32_t sessionID, std::shared_ptr<SessionState> state);
// Lookup a session entry by session ID // Lookup a session entry by session ID
std::shared_ptr<const SessionState> get(uint32_t sessionID) const; std::shared_ptr<SessionState> get(uint32_t sessionID) const;
// Lookup a session entry by IPv4 // Lookup a session entry by IPv4
std::shared_ptr<const SessionState> getByIP(uint32_t ip) const; std::shared_ptr<SessionState> getByIP(uint32_t ip) const;
// Get a snapshot of the Session Registry // Get a snapshot of the Session Registry
std::unordered_map<uint32_t, std::shared_ptr<SessionState>> snapshot() const; std::unordered_map<uint32_t, std::shared_ptr<SessionState>> snapshot() const;

View File

@@ -53,6 +53,9 @@ namespace ColumnLynx::Net::TCP {
: :
mHandler(std::make_shared<MessageHandler>(std::move(socket))), mHandler(std::make_shared<MessageHandler>(std::move(socket))),
mHeartbeatTimer(mHandler->socket().get_executor()), mHeartbeatTimer(mHandler->socket().get_executor()),
mConnectionAESKey{},
mConnectionSessionID(0),
mConnectionPublicKey{},
mLastHeartbeatReceived(std::chrono::steady_clock::now()), mLastHeartbeatReceived(std::chrono::steady_clock::now()),
mLastHeartbeatSent(std::chrono::steady_clock::now()) mLastHeartbeatSent(std::chrono::steady_clock::now())
{} {}

View File

@@ -9,6 +9,7 @@
#include <unordered_map> #include <unordered_map>
#include <vector> #include <vector>
#include <memory> #include <memory>
#include <mutex>
#include <unordered_set> #include <unordered_set>
#include <new> #include <new>
#include <asio.hpp> #include <asio.hpp>
@@ -77,6 +78,7 @@ namespace ColumnLynx::Net::TCP {
asio::io_context &mIoContext; asio::io_context &mIoContext;
asio::ip::tcp::acceptor mAcceptor; asio::ip::tcp::acceptor mAcceptor;
std::mutex mClientsMutex;
std::unordered_set<TCPConnection::pointer> mClients; std::unordered_set<TCPConnection::pointer> mClients;
}; };

View File

@@ -184,7 +184,7 @@ int main(int argc, char** argv) {
debug("isDone flag: " + std::to_string(done)); debug("isDone flag: " + std::to_string(done));
// Client is running // Client is running
while ((client->isConnected() || !client->isHandshakeComplete()) && !done) { while (client->isConnected() && !done) {
//debug("Client connection flag: " + std::to_string(client->isConnected())); //debug("Client connection flag: " + std::to_string(client->isConnected()));
auto packet = tun->readPacket(); auto packet = tun->readPacket();
/*if (!client->isConnected() || done) { /*if (!client->isConnected() || done) {

View File

@@ -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())); 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) { void UDPClient::sendMessage(const std::string& data) {

View File

@@ -7,17 +7,22 @@
namespace ColumnLynx::Net { namespace ColumnLynx::Net {
void SessionRegistry::put(uint32_t sessionID, std::shared_ptr<SessionState> state) { void SessionRegistry::put(uint32_t sessionID, std::shared_ptr<SessionState> state) {
std::unique_lock lock(mMutex); 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); mSessions[sessionID] = std::move(state);
mIPSessions[mSessions[sessionID]->clientTunIP] = mSessions[sessionID]; mIPSessions[mSessions[sessionID]->clientTunIP] = mSessions[sessionID];
} }
std::shared_ptr<const SessionState> SessionRegistry::get(uint32_t sessionID) const { std::shared_ptr<SessionState> SessionRegistry::get(uint32_t sessionID) const {
std::shared_lock lock(mMutex); std::shared_lock lock(mMutex);
auto it = mSessions.find(sessionID); auto it = mSessions.find(sessionID);
return (it == mSessions.end()) ? nullptr : it->second; return (it == mSessions.end()) ? nullptr : it->second;
} }
std::shared_ptr<const SessionState> SessionRegistry::getByIP(uint32_t ip) const { std::shared_ptr<SessionState> SessionRegistry::getByIP(uint32_t ip) const {
std::shared_lock lock(mMutex); std::shared_lock lock(mMutex);
auto it = mIPSessions.find(ip); auto it = mIPSessions.find(ip);
return (it == mIPSessions.end()) ? nullptr : it->second; 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 { uint32_t SessionRegistry::getFirstAvailableIP(uint32_t baseIP, uint8_t mask) const {
std::shared_lock lock(mMutex); std::shared_lock lock(mMutex);
if (mask == 0 || mask >= 32) return 0; // degenerate subnet
uint32_t hostCount = (1u << (32 - mask)); uint32_t hostCount = (1u << (32 - mask));
if (hostCount < 3) return 0; // no assignable host addresses
uint32_t firstHost = 2; uint32_t firstHost = 2;
uint32_t lastHost = hostCount - 2; uint32_t lastHost = hostCount - 2;

View File

@@ -54,6 +54,12 @@ namespace ColumnLynx::Net::TCP {
mCurrentType = decodeMessageType(mHeader[0]); mCurrentType = decodeMessageType(mHeader[0]);
uint16_t len = (mHeader[1] << 8) | mHeader[2]; 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); mReadBody(len);
} else { } else {
if (!NetHelper::isExpectedDisconnect(ec)) { if (!NetHelper::isExpectedDisconnect(ec)) {

View File

@@ -87,7 +87,7 @@ namespace ColumnLynx::Utils {
} }
std::string getVersion() { std::string getVersion() {
return "1.2.0"; return "1.2.1";
} }
unsigned short serverPort() { unsigned short serverPort() {

View File

@@ -58,6 +58,44 @@ static void InitializeWintun()
#undef RESOLVE #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<std::string>& 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 #endif // _WIN32
namespace ColumnLynx::Net { namespace ColumnLynx::Net {
@@ -116,8 +154,10 @@ static bool runCommand(const std::vector<std::string>& args) {
struct ctl_info ctlInfo {}; struct ctl_info ctlInfo {};
std::strncpy(ctlInfo.ctl_name, UTUN_CONTROL_NAME, sizeof(ctlInfo.ctl_name)); 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))); throw std::runtime_error("ioctl(CTLIOCGINFO) failed: " + std::string(strerror(errno)));
}
struct sockaddr_ctl sc {}; struct sockaddr_ctl sc {};
sc.sc_len = sizeof(sc); sc.sc_len = sizeof(sc);
@@ -127,9 +167,11 @@ static bool runCommand(const std::vector<std::string>& args) {
sc.sc_unit = 0; // 0 = auto-assign next utunX sc.sc_unit = 0; // 0 = auto-assign next utunX
if (connect(mFd, (struct sockaddr*)&sc, sizeof(sc)) < 0) { 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: 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 // Retrieve actual utun device name via UTUN_OPT_IFNAME
@@ -351,20 +393,10 @@ static bool runCommand(const std::vector<std::string>& args) {
//); //);
//system(cmd); //system(cmd);
#elif defined(_WIN32) #elif defined(_WIN32)
char cmd[512];
// Remove any persistent routes associated with this interface // Remove any persistent routes associated with this interface
snprintf(cmd, sizeof(cmd), runCommandWin32({"netsh", "routing", "ip", "delete", "persistentroute", "all", "name=" + mIfName});
"netsh routing ip delete persistentroute all name=\"%s\"",
mIfName.c_str()
);
system(cmd);
// Reset to DHCP // Reset to DHCP
snprintf(cmd, sizeof(cmd), runCommandWin32({"netsh", "interface", "ip", "set", "address", "name=" + mIfName, "dhcp"});
"netsh interface ip set address name=\"%s\" dhcp",
mIfName.c_str()
);
system(cmd);
#endif #endif
} }

View File

@@ -229,17 +229,32 @@ namespace ColumnLynx::Net::TCP {
std::memcpy(mConnectionAESKey.data(), decrypted.data(), decrypted.size()); std::memcpy(mConnectionAESKey.data(), decrypted.data(), decrypted.size());
// Make a Session ID - unique and not zero (zero is reserved for invalid sessions) // Make a Session ID - unique and not zero (zero is reserved for invalid sessions)
do { {
randombytes_buf(&mConnectionSessionID, sizeof(mConnectionSessionID)); int idAttempts = 0;
} while (SessionRegistry::getInstance().exists(mConnectionSessionID) || mConnectionSessionID == 0); // Regenerate if it already exists or is zero (zero is reserved for invalid sessions) 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) // Encrypt the Session ID with the established AES key (using symmetric encryption, nonce can be all zeros for this purpose)
Nonce symNonce{}; // All zeros Nonce symNonce{}; // All zeros
const auto& serverConfig = ServerSession::getInstance().getRawServerConfig(); const auto& serverConfig = ServerSession::getInstance().getRawServerConfig();
std::string networkString = serverConfig.find("NETWORK")->second; // The load check guarantees that this value exists auto netIt = serverConfig.find("NETWORK");
uint8_t configMask = std::stoi(serverConfig.find("SUBNET_MASK")->second); // Same deal here 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<uint8_t>(std::stoi(maskIt->second));
uint32_t baseIP = Net::VirtualInterface::stringToIpv4(networkString); uint32_t baseIP = Net::VirtualInterface::stringToIpv4(networkString);
@@ -281,7 +296,7 @@ namespace ColumnLynx::Net::TCP {
// Add to session registry // Add to session registry
Utils::log("Handshake with " + reqAddr + " completed successfully. Session ID assigned (" + std::to_string(mConnectionSessionID) + ")."); Utils::log("Handshake with " + reqAddr + " completed successfully. Session ID assigned (" + std::to_string(mConnectionSessionID) + ").");
auto session = std::make_shared<SessionState>(mConnectionAESKey, std::chrono::hours(12), clientIP, htonl(0x0A0A0001), mConnectionSessionID); auto session = std::make_shared<SessionState>(mConnectionAESKey, std::chrono::hours(12), clientIP, htonl(baseIP + 1), mConnectionSessionID);
session->setBaseNonce(); // Set it session->setBaseNonce(); // Set it
SessionRegistry::getInstance().put(mConnectionSessionID, std::move(session)); SessionRegistry::getInstance().put(mConnectionSessionID, std::move(session));
SessionRegistry::getInstance().lockIP(mConnectionSessionID, clientIP); SessionRegistry::getInstance().lockIP(mConnectionSessionID, clientIP);

View File

@@ -35,11 +35,15 @@ namespace ColumnLynx::Net::TCP {
auto client = TCPConnection::create( auto client = TCPConnection::create(
std::move(socket), std::move(socket),
[this](std::shared_ptr<TCPConnection> c) { [this](std::shared_ptr<TCPConnection> c) {
std::lock_guard<std::mutex> lock(mClientsMutex);
mClients.erase(c); mClients.erase(c);
Utils::log("Client removed."); Utils::log("Client removed.");
} }
); );
mClients.insert(client); {
std::lock_guard<std::mutex> lock(mClientsMutex);
mClients.insert(client);
}
client->start(); client->start();
Utils::log("Accepted new client connection."); Utils::log("Accepted new client connection.");
@@ -59,7 +63,11 @@ namespace ColumnLynx::Net::TCP {
} }
// Snapshot to avoid iterator invalidation while callbacks erase() // Snapshot to avoid iterator invalidation while callbacks erase()
std::vector<std::shared_ptr<TCPConnection>> snapshot(mClients.begin(), mClients.end()); std::vector<std::shared_ptr<TCPConnection>> snapshot;
{
std::lock_guard<std::mutex> lock(mClientsMutex);
snapshot.assign(mClients.begin(), mClients.end());
}
for (auto &client : snapshot) { for (auto &client : snapshot) {
try { try {
client->disconnect(); // should shutdown+close the socket client->disconnect(); // should shutdown+close the socket

View File

@@ -40,7 +40,7 @@ namespace ColumnLynx::Net::UDP {
std::vector<uint8_t> encryptedPayload(it, mRecvBuffer.begin() + bytes); std::vector<uint8_t> encryptedPayload(it, mRecvBuffer.begin() + bytes);
// Get associated session state // Get associated session state
std::shared_ptr<const SessionState> session = SessionRegistry::getInstance().get(sessionID); std::shared_ptr<SessionState> session = SessionRegistry::getInstance().get(sessionID);
if (!session) { if (!session) {
Utils::warn("UDP: Unknown or invalid session from " + mRemoteEndpoint.address().to_string()); Utils::warn("UDP: Unknown or invalid session from " + mRemoteEndpoint.address().to_string());
@@ -60,9 +60,9 @@ namespace ColumnLynx::Net::UDP {
Utils::debug("Passed decryption"); Utils::debug("Passed decryption");
const_cast<SessionState*>(session.get())->setUDPEndpoint(mRemoteEndpoint); // Update endpoint after confirming decryption session->setUDPEndpoint(mRemoteEndpoint); // Update endpoint after confirming decryption
// Update recv counter // Update recv counter
const_cast<SessionState*>(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 // For now, just log the decrypted payload
std::string payloadStr(plaintext.begin(), plaintext.end()); 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) { void UDPServer::sendData(uint32_t sessionID, const std::string& data) {
// Find the IPv4/IPv6 endpoint for the session // Find the IPv4/IPv6 endpoint for the session
std::shared_ptr<const SessionState> session = SessionRegistry::getInstance().get(sessionID); std::shared_ptr<SessionState> session = SessionRegistry::getInstance().get(sessionID);
if (!session) { if (!session) {
Utils::warn("UDP: Cannot send data, unknown session ID " + std::to_string(sessionID)); Utils::warn("UDP: Cannot send data, unknown session ID " + std::to_string(sessionID));
return; return;
@@ -98,14 +98,13 @@ namespace ColumnLynx::Net::UDP {
// Increment send counter with overflow protection // Increment send counter with overflow protection
uint64_t sendCount = 0; uint64_t sendCount = 0;
{ {
auto ptr = const_cast<SessionState*>(session.get()); uint64_t old = session->send_ctr.load(std::memory_order_relaxed);
uint64_t old = ptr->send_ctr.load(std::memory_order_relaxed);
for (;;) { for (;;) {
if (old == std::numeric_limits<uint64_t>::max()) { if (old == std::numeric_limits<uint64_t>::max()) {
Utils::error("UDP: send counter overflow for session " + std::to_string(sessionID)); Utils::error("UDP: send counter overflow for session " + std::to_string(sessionID));
return; 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; sendCount = old;
break; break;
} }