From ffeabec42c62d573af1e0a2d2371493a5bfec871 Mon Sep 17 00:00:00 2001 From: Vlad Khorsun Date: Thu, 16 Jul 2026 16:57:56 +0300 Subject: [PATCH 1/2] Fixed bug #9094 : FB4 XNET/local attach stack-overflows in fbclient (gds__log recursion) when the calling thread impersonates another user's token Tried to preserve compatibility with WinXP, fortunately it is not required for v5+. Also, correct checks for mutex existence. --- src/common/isc.cpp | 122 +++++++++++++++++++++++++++++++++++++++------ src/yvalve/gds.cpp | 6 +-- 2 files changed, 109 insertions(+), 19 deletions(-) diff --git a/src/common/isc.cpp b/src/common/isc.cpp index f99bcbba5f9..01e5e286298 100644 --- a/src/common/isc.cpp +++ b/src/common/isc.cpp @@ -61,21 +61,110 @@ #include #include +// Check if Windows SDK version is above v7 +#include +#ifdef _WIN32_WINNT_WIN8 +#include +#endif + +// Before Vista, pseudo-handle returned by GetCurrentProcess() could contain not enough +// privileges to change security descriptor. Thus use hard way to obtain good for us handle. +static HANDLE legacyGetCurrentProcess() +{ + HANDLE hCurrentProcess = NULL; + + // If current thread impersonates some low-privileged account, it might have + // no access to open handle of current process + + HANDLE hToken = NULL; + DWORD err = 0; + if (OpenThreadToken(GetCurrentThread(), TOKEN_IMPERSONATE | TOKEN_DUPLICATE, TRUE, &hToken)) + { + if (RevertToSelf()) + { + hCurrentProcess = OpenProcess(READ_CONTROL | WRITE_DAC, FALSE, GetCurrentProcessId()); + err = GetLastError(); + SetThreadToken(NULL, hToken); + } + else + { + // MSDN: You should shut down the process if RevertToSelf fails. + Firebird::string err; + err.printf("RevertToSelf failed with %d", GetLastError()); + gds__log(err.c_str()); + + Firebird::fatal_exception::raise(err.c_str()); + } + CloseHandle(hToken); + } + else + { + if (GetLastError() == ERROR_NO_TOKEN) + { + // No impersonation + hCurrentProcess = OpenProcess(READ_CONTROL | WRITE_DAC, FALSE, GetCurrentProcessId()); + err = GetLastError(); + } + else + { + // Impersonated account have too low privileges + Firebird::system_call_failed::raise("OpenThreadToken"); + } + } + + if (hCurrentProcess == NULL) + Firebird::system_call_failed::raise("OpenProcess", err); + + return hCurrentProcess; +} + + class SecurityAttributes { public: explicit SecurityAttributes(MemoryPool& pool) : m_pool(pool) { + static bool initializing = false; + + // initializing == true means recursive call of SecurityAttributes constructor. + // It happens if first instance throws an exception and handling code accesses + // SecurityAttributes. Avoid recursion and leave null at attributes.lpSecurityDescriptor + // in this case. + + if (initializing) + return; + + initializing = true; + + attributes.nLength = sizeof(attributes); + attributes.lpSecurityDescriptor = NULL; + attributes.bInheritHandle = TRUE; + // Ensure that our process has the SYNCHRONIZE privilege granted to everyone PSECURITY_DESCRIPTOR pOldSD = NULL; PACL pOldACL = NULL; // Pseudo-handles do not work on WinNT. Need real process handle. - HANDLE hCurrentProcess = OpenProcess(READ_CONTROL | WRITE_DAC, FALSE, GetCurrentProcessId()); - if (hCurrentProcess == NULL) { - Firebird::system_call_failed::raise("OpenProcess"); + // hvlad: it have PROCESS_ALL_ACCESS access right since Vista + +#ifdef _WIN32_WINNT_WIN8 + const bool isVistaOrGreater = IsWindowsVistaOrGreater(); +#else + OSVERSIONINFO OsVersionInfo; + OsVersionInfo.dwOSVersionInfoSize = sizeof(OSVERSIONINFO); + + const bool isVistaOrGreater = GetVersionEx(&OsVersionInfo) && + (OsVersionInfo.dwMajorVersion >= 6); +#endif + HANDLE hCurrentProcess = NULL; + if (isVistaOrGreater) + { + // Pseudo handle is not affected by impersonation and contains PROCESS_ALL_ACCESS access right + hCurrentProcess = GetCurrentProcess(); } + else + hCurrentProcess = legacyGetCurrentProcess(); DWORD result = GetSecurityInfo(hCurrentProcess, SE_KERNEL_OBJECT, DACL_SECURITY_INFORMATION, NULL, NULL, &pOldACL, NULL, &pOldSD); @@ -89,7 +178,9 @@ class SecurityAttributes if (result != ERROR_SUCCESS) { - CloseHandle(hCurrentProcess); + if (!isVistaOrGreater) + CloseHandle(hCurrentProcess); + Firebird::system_call_failed::raise("GetSecurityInfo", result); } @@ -117,19 +208,18 @@ class SecurityAttributes SetSecurityInfo(hCurrentProcess, SE_KERNEL_OBJECT, DACL_SECURITY_INFORMATION, NULL, NULL, pNewACL, NULL); - if (pSID) { + if (pSID) FreeSid(pSID); - } - if (pNewACL) { + + if (pNewACL) LocalFree(pNewACL); - } } - CloseHandle(hCurrentProcess); + if (!isVistaOrGreater) + CloseHandle(hCurrentProcess); - if (pOldSD) { + if (pOldSD) LocalFree(pOldSD); - } // Create and initialize the default security descriptor // to be assigned to various IPC objects. @@ -140,16 +230,16 @@ class SecurityAttributes PSECURITY_DESCRIPTOR p_security_desc = static_cast( FB_NEW_POOL(m_pool) char[SECURITY_DESCRIPTOR_MIN_LENGTH]); - attributes.nLength = sizeof(attributes); - attributes.lpSecurityDescriptor = p_security_desc; - attributes.bInheritHandle = TRUE; - if (!InitializeSecurityDescriptor(p_security_desc, SECURITY_DESCRIPTOR_REVISION) || !SetSecurityDescriptorDacl(p_security_desc, TRUE, NULL, FALSE)) { delete p_security_desc; - attributes.lpSecurityDescriptor = NULL; } + else + { + attributes.lpSecurityDescriptor = p_security_desc; + } + initializing = false; } ~SecurityAttributes() diff --git a/src/yvalve/gds.cpp b/src/yvalve/gds.cpp index e3c7373bd45..e5fea3aaf63 100644 --- a/src/yvalve/gds.cpp +++ b/src/yvalve/gds.cpp @@ -1026,10 +1026,10 @@ class LogFileHandles ~LogFileHandles() { - if (mutex_handle != INVALID_HANDLE_VALUE) + if (mutex_handle != NULL) CloseHandle(mutex_handle); - mutex_handle = INVALID_HANDLE_VALUE; + mutex_handle = NULL; if (file_handle != INVALID_HANDLE_VALUE) CloseHandle(file_handle); @@ -1088,7 +1088,7 @@ void LogFileHandles::trace_raw(const char* text, unsigned int length) Firebird::InitInstance logFileHandles; -HANDLE LogFileHandles::mutex_handle = INVALID_HANDLE_VALUE; +HANDLE LogFileHandles::mutex_handle = NULL; HANDLE LogFileHandles::file_handle = INVALID_HANDLE_VALUE; From b7efbd2772a076eb21b1cfbb1476d030e6fbb697 Mon Sep 17 00:00:00 2001 From: Vlad Khorsun Date: Thu, 16 Jul 2026 17:32:37 +0300 Subject: [PATCH 2/2] Replace Windows 2025 (VS 2026) runner by Windows 2022 (VS 2022). --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 895dee91c0c..8331623f602 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -164,7 +164,7 @@ jobs: fail-fast: false matrix: os: - - windows-2025 + - windows-2022 platform: [x64, x86] steps: