From cdb963e70b64b3af23499e6f7b6be92947c69a3c Mon Sep 17 00:00:00 2001 From: Tony Mountifield Date: Tue, 11 Aug 2026 21:21:37 +0100 Subject: [PATCH 1/3] Rework the binding of IPv4 and IPv6 sockets This corrects logic errors for bind() failure handling found by AI. bIPv6Available is now only set if bind() succeeds for IPv6. --- src/socket.cpp | 75 +++++++++++++++++++++++++++++++++++++------------- 1 file changed, 56 insertions(+), 19 deletions(-) diff --git a/src/socket.cpp b/src/socket.cpp index e95ab4a41f..40f82d2ad8 100644 --- a/src/socket.cpp +++ b/src/socket.cpp @@ -290,8 +290,6 @@ void CSocket::Init ( const quint16 iNewPortNumber, } } - bIPv6Available = true; // this is a reference to CClient::bIPv6Available or CServer::bIPv6Available - // set socket to non-blocking #ifdef _WIN32 unsigned long mode = 1; @@ -318,20 +316,27 @@ void CSocket::Init ( const quint16 iNewPortNumber, vecbyRecBuf.Init ( MAX_SIZE_BYTES_NETW_BUF ); // initialize the listening socket - bool bSuccess; + bool bSuccess = false; // will become true if IPv4 bind succeeds if ( bIsClient ) { + // for a client, it does not matter if the IPv4 and IPv6 sockets get bound + // to different local port numbers if ( iPortNumber == 0 ) { // if port number is 0, bind the client to a random available port sa4.sin_port = sa6.sin6_port = htons ( 0 ); - bSuccess = ( ::bind ( UdpSocket4, (struct sockaddr*) &sa4, sa4len ) == 0 ); + bSuccess = ( ::bind ( UdpSocket4, (struct sockaddr*) &sa4, sa4len ) != -1 ); - if ( UdpSocket6 != INVALID_SOCKET ) + // only try to bind the IPv6 socket if IPv4 has succeeded and IPv6 socket is open + if ( bSuccess && UdpSocket6 != INVALID_SOCKET ) { - bSuccess = bSuccess && ( ::bind ( UdpSocket6, (struct sockaddr*) &sa6, sa6len ) == 0 ); + if ( ::bind ( UdpSocket6, (struct sockaddr*) &sa6, sa6len ) != -1 ) + { + // note that IPv6 is available + bIPv6Available = true; // this is a reference to CClient::bIPv6Available or CServer::bIPv6Available + } } } else @@ -341,23 +346,38 @@ void CSocket::Init ( const quint16 iNewPortNumber, // faulty router gets stuck and confused by a particular port (like // the starting port). Might work around frustrating "cannot connect" // problems (#568) - const quint16 startingPortNumber = iPortNumber + rand() % NUM_SOCKET_PORTS_TO_TRY; - - quint16 iClientPortIncrement = 0; - bSuccess = false; // initialization for while loop + const quint32 startingPortNumber = static_cast ( iPortNumber ) + rand() % NUM_SOCKET_PORTS_TO_TRY; - while ( !bSuccess && ( iClientPortIncrement <= NUM_SOCKET_PORTS_TO_TRY ) ) + for ( quint32 port = startingPortNumber; port < startingPortNumber + NUM_SOCKET_PORTS_TO_TRY; port++ ) { - sa4.sin_port = sa6.sin6_port = htons ( startingPortNumber + iClientPortIncrement ); + // do not overflow 16-bit port number + if ( port > 65535U ) + { + break; + } - bSuccess = ( ::bind ( UdpSocket4, (struct sockaddr*) &sa4, sa4len ) == 0 ); + // bind IPv4 socket if not bound + if ( !bSuccess ) + { + sa4.sin_port = htons ( port ); + bSuccess = ( ::bind ( UdpSocket4, (struct sockaddr*) &sa4, sa4len ) != -1 ); + } - if ( UdpSocket6 != INVALID_SOCKET ) + // only try to bind the IPv6 socket if IPv4 has succeeded and IPv6 socket is open + if ( bSuccess && UdpSocket6 != INVALID_SOCKET && !bIPv6Available ) { - bSuccess = bSuccess && ( ::bind ( UdpSocket6, (struct sockaddr*) &sa6, sa6len ) == 0 ); + sa6.sin6_port = htons ( port ); + if ( ::bind ( UdpSocket6, (struct sockaddr*) &sa6, sa6len ) != -1 ) + { + // note that IPv6 is available + bIPv6Available = true; // this is a reference to CClient::bIPv6Available or CServer::bIPv6Available + } } - iClientPortIncrement++; + if ( bSuccess && ( bIPv6Available || UdpSocket6 == INVALID_SOCKET ) ) + { + break; + } } } } @@ -369,14 +389,31 @@ void CSocket::Init ( const quint16 iNewPortNumber, sa4.sin_port = sa6.sin6_port = htons ( iPortNumber ); - bSuccess = ( ::bind ( UdpSocket4, (struct sockaddr*) &sa4, sa4len ) == 0 ); + bSuccess = ( ::bind ( UdpSocket4, (struct sockaddr*) &sa4, sa4len ) != -1 ); - if ( UdpSocket6 != INVALID_SOCKET ) + // only try to bind the IPv6 socket if IPv4 has succeeded and IPv6 socket is open + if ( bSuccess && UdpSocket6 != INVALID_SOCKET ) { - bSuccess = bSuccess && ( ::bind ( UdpSocket6, (struct sockaddr*) &sa6, sa6len ) == 0 ); + if ( ::bind ( UdpSocket6, (struct sockaddr*) &sa6, sa6len ) != -1 ) + { + // note that IPv6 is available + bIPv6Available = true; // this is a reference to CClient::bIPv6Available or CServer::bIPv6Available + } } } + if ( UdpSocket6 != INVALID_SOCKET && !bIPv6Available ) + { + // IPv6 bind failed - don't cancel bSuccess, but close the IPv6 socket +#ifdef _WIN32 + closesocket ( UdpSocket6 ); +#else + close ( UdpSocket6 ); +#endif + UdpSocket6 = INVALID_SOCKET; + qWarning() << "IPv6 socket closed - failed to bind"; + } + if ( !bSuccess ) { // we cannot bind socket, throw error From 0ab2d13153bbff9e884e6219373cd37e296a1739 Mon Sep 17 00:00:00 2001 From: Tony Mountifield Date: Tue, 11 Aug 2026 22:17:47 +0100 Subject: [PATCH 2/3] Use Qt random number generator for starting port in client Previously, rand() was used and is always seeded with 1, providing no randomness at program start. --- src/socket.cpp | 2 +- src/socket.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/socket.cpp b/src/socket.cpp index 40f82d2ad8..a841fefb39 100644 --- a/src/socket.cpp +++ b/src/socket.cpp @@ -346,7 +346,7 @@ void CSocket::Init ( const quint16 iNewPortNumber, // faulty router gets stuck and confused by a particular port (like // the starting port). Might work around frustrating "cannot connect" // problems (#568) - const quint32 startingPortNumber = static_cast ( iPortNumber ) + rand() % NUM_SOCKET_PORTS_TO_TRY; + const quint32 startingPortNumber = static_cast ( iPortNumber ) + QRandomGenerator::global()->bounded ( NUM_SOCKET_PORTS_TO_TRY ); for ( quint32 port = startingPortNumber; port < startingPortNumber + NUM_SOCKET_PORTS_TO_TRY; port++ ) { diff --git a/src/socket.h b/src/socket.h index 57f005b2bf..5000edcbb0 100644 --- a/src/socket.h +++ b/src/socket.h @@ -49,6 +49,7 @@ #include #include #include +#include #include #include #include "global.h" From 05b4103834798e3c7493c4a1d06640a90658f250 Mon Sep 17 00:00:00 2001 From: Tony Mountifield Date: Wed, 12 Aug 2026 15:43:50 +0100 Subject: [PATCH 3/3] Clear bIPv6Available after closing sockets in Init() --- src/socket.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/socket.cpp b/src/socket.cpp index a841fefb39..23b85c40a4 100644 --- a/src/socket.cpp +++ b/src/socket.cpp @@ -172,6 +172,8 @@ void CSocket::Init ( const quint16 iNewPortNumber, UdpSocket6 = INVALID_SOCKET; } + bIPv6Available = false; // re-init before opening sockets + struct sockaddr_in sa4; socklen_t sa4len = sizeof ( sa4 ); memset ( &sa4, 0, sa4len );