From 9d52166d285c0ab254ef3176b0434ecb0f69e129 Mon Sep 17 00:00:00 2001 From: WangYi Date: Wed, 27 Aug 2025 03:37:51 +0800 Subject: [PATCH 1/2] fix(gige): Fix use-after-free in asynchronous GVCP logging The function inet_ntoa returns a pointer to a temporary, thread-local static buffer. In GVCPClient::openClientSocket, this temporary pointer was being passed directly to the asynchronous LOG_INTVL macro. This created a race condition where the background logging thread could attempt to access the pointer after the static buffer's content was no longer valid (e.g., after the calling thread exited). This resulted in a heap-use-after-free error, which was reliably detected by AddressSanitizer. The fix is to wrap the call to inet_ntoa in std::string(), ensuring that a copy of the string data is made immediately. This safe copy is then passed to the asynchronous logger, resolving the lifetime issue. --- src/platform/ethernet/gige/GVCPClient.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform/ethernet/gige/GVCPClient.cpp b/src/platform/ethernet/gige/GVCPClient.cpp index 4ad5b8af..940f873e 100644 --- a/src/platform/ethernet/gige/GVCPClient.cpp +++ b/src/platform/ethernet/gige/GVCPClient.cpp @@ -335,7 +335,7 @@ SOCKET GVCPClient::openClientSocket(SOCKADDR_IN addr) { #if(defined(__linux__) || defined(OS_IOS) || defined(OS_MACOS) || defined(__ANDROID__)) //addr.sin_addr.s_addr = inet_addr("0.0.0.0"); #endif - LOG_INTVL(LOG_INTVL_OBJECT_TAG + "GVCP bind", MAX_LOG_INTERVAL, spdlog::level::debug, "bind {}:{}", inet_ntoa(addr.sin_addr), ntohs(addr.sin_port)); + LOG_INTVL(LOG_INTVL_OBJECT_TAG + "GVCP bind", MAX_LOG_INTERVAL, spdlog::level::debug, "bind {}:{}", std::string(inet_ntoa(addr.sin_addr)), ntohs(addr.sin_port)); err = bind(sock, (SOCKADDR *)&addr, sizeof(SOCKADDR)); if(err == SOCKET_ERROR) { return 0; From 4b2ba7beb3411c245a0a494f8a8705772b8ff8d9 Mon Sep 17 00:00:00 2001 From: WangYi Date: Tue, 9 Sep 2025 21:08:54 +0800 Subject: [PATCH 2/2] Fix source IP selection in VendorTCPClient for multi-IP Windows adapters - Prevent overwriting caller-provided localAddress_ in checkLocalIP() - Prioritize subnet-matching IP for destination address - Fallback to first private IPv4 if no match, or clear for OS decision - Ensure correct bind() in socketConnect() to avoid wrong source IP in packets - Addresses issue where packets were sent from wrong IP (e.g., 192.168.1.107 instead of 192.168.2.165) on adapters with multiple IPs --- .../ethernet/socket/VendorTCPClient.cpp | 108 +++++++++++------- 1 file changed, 66 insertions(+), 42 deletions(-) diff --git a/src/platform/ethernet/socket/VendorTCPClient.cpp b/src/platform/ethernet/socket/VendorTCPClient.cpp index f8c208eb..c53e4c39 100644 --- a/src/platform/ethernet/socket/VendorTCPClient.cpp +++ b/src/platform/ethernet/socket/VendorTCPClient.cpp @@ -48,57 +48,81 @@ VendorTCPClient::VendorTCPClient(std::string localAddress, std::string localMac, } void VendorTCPClient::checkLocalIP() { -#if(defined(WIN32) || defined(_WIN32) || defined(WINCE)) - unsigned int a, b, c, d; - if(sscanf(localAddress_.c_str(), "%u.%u.%u.%u", &a, &b, &c, &d) == 4) { - if((a == 10) || // 10.0.0.0 - 10.255.255.255 - (a == 172 && b >= 16 && b <= 31) || // 172.16.0.0 - 172.31.255.255 - (a == 192 && b == 168)) { // 192.168.0.0 - 192.168.255.255 - isValidIP_ = true; - } - } +#if (defined(WIN32) || defined(_WIN32) || defined(WINCE)) + // If we already have a private localAddress_ provided, try to keep it + auto isPrivate = [](const std::string &ip) -> bool { + unsigned a,b,c,d; + if (sscanf(ip.c_str(), "%u.%u.%u.%u", &a,&b,&c,&d) != 4) return false; + return (a == 10) || (a == 172 && b >= 16 && b <= 31) || (a == 192 && b == 168); + }; + + // Parse destination IP for subnet matching + in_addr destAddr{}; + bool hasDest = (inet_pton(AF_INET, address_.c_str(), &destAddr) == 1); + + // Enumerate adapter by MAC and pick the right IP + IP_ADAPTER_INFO adapterInfo[16]; + DWORD bufLen = sizeof(adapterInfo); + DWORD dwRetVal = GetAdaptersInfo(adapterInfo, &bufLen); + if (dwRetVal == ERROR_SUCCESS) { + PIP_ADAPTER_INFO pAdapterInfo = adapterInfo; + while (pAdapterInfo) { + // Build MAC string for comparison + std::ostringstream oss; + for (int i = 0; i < 6; ++i) { + if (i > 0) oss << ":"; + oss << std::setw(2) << std::setfill('0') << std::hex << (int)pAdapterInfo->Address[i]; + } + std::string currentMac = oss.str(); + + if (currentMac == localMac_) { + // First: if caller-specified localAddress_ exists on this adapter, keep it + if (!localAddress_.empty()) { + for (PIP_ADDR_STRING p = &pAdapterInfo->IpAddressList; p; p = p->Next) { + if (localAddress_ == p->IpAddress.String) { + isValidIP_ = isPrivate(localAddress_); + return; // keep caller-provided address + } + } + } - if(!isValidIP_) { - localAddress_ = ""; - IP_ADAPTER_INFO adapterInfo[16]; // Buffer to hold adapter info - DWORD bufLen = sizeof(adapterInfo); - - // Get the network adapter information - DWORD dwRetVal = GetAdaptersInfo(adapterInfo, &bufLen); - if(dwRetVal == ERROR_SUCCESS) { - // Loop through all the adapters - PIP_ADAPTER_INFO pAdapterInfo = adapterInfo; - while(pAdapterInfo) { - // Convert MAC address to string format - std::ostringstream oss; - for(int i = 0; i < 6; ++i) { - if(i > 0) - oss << ":"; - oss << std::setw(2) << std::setfill('0') << std::hex << (int)pAdapterInfo->Address[i]; + // Second: try to pick an IP on same subnet as destination + if (hasDest) { + for (PIP_ADDR_STRING p = &pAdapterInfo->IpAddressList; p; p = p->Next) { + in_addr ip{}, mask{}; + if (inet_pton(AF_INET, p->IpAddress.String, &ip) != 1) continue; + if (inet_pton(AF_INET, p->IpMask.String, &mask) != 1) continue; + if ( (ip.S_un.S_addr & mask.S_un.S_addr) == (destAddr.S_un.S_addr & mask.S_un.S_addr) ) { + localAddress_ = p->IpAddress.String; + isValidIP_ = isPrivate(localAddress_); + return; + } + } } - // Compare MAC address with the input MAC address - std::string currentMac = oss.str(); - if(currentMac == localMac_) { - // If MAC matches, get the associated IP address (return the first one) - PIP_ADDR_STRING pIpAddr = &pAdapterInfo->IpAddressList; - if(pIpAddr) { - localAddress_ = pIpAddr->IpAddress.String; // Return the first IP address + // Third: fallback to the first private IPv4 on this adapter + for (PIP_ADDR_STRING p = &pAdapterInfo->IpAddressList; p; p = p->Next) { + if (isPrivate(p->IpAddress.String)) { + localAddress_ = p->IpAddress.String; + isValidIP_ = true; + return; } - break; // If no IP address found, break out of the loop } - pAdapterInfo = pAdapterInfo->Next; - } - } - if(sscanf(localAddress_.c_str(), "%u.%u.%u.%u", &a, &b, &c, &d) == 4) { - if((a == 10) || // 10.0.0.0 - 10.255.255.255 - (a == 172 && b >= 16 && b <= 31) || // 172.16.0.0 - 172.31.255.255 - (a == 192 && b == 168)) { // 192.168.0.0 - 192.168.255.255 - isValidIP_ = true; + // No suitable IP; clear to let OS decide + localAddress_.clear(); + isValidIP_ = false; + return; } + pAdapterInfo = pAdapterInfo->Next; } } + + // Adapter not found or error: if caller provided a private IP, keep it; otherwise clear + isValidIP_ = isPrivate(localAddress_); + if (!isValidIP_) localAddress_.clear(); +#else + // Non-Windows: keep current behavior (if any) #endif }