From 057b25135848575d6f62eed97874a8d71e72af1d Mon Sep 17 00:00:00 2001 From: 11soda11 <115734183+Sodastream11@users.noreply.github.com> Date: Wed, 29 Apr 2026 00:13:15 +0200 Subject: [PATCH] tool_doswin: add stdin relay auth Verify the peer with a random number in the background thread. Closes #21467 --- docs/KNOWN_BUGS.md | 15 ------ lib/curlx/winapi.c | 20 ++++++++ lib/curlx/winapi.h | 1 + lib/rand.c | 20 +------- lib/rand.h | 6 --- lib/vtls/schannel.c | 4 +- projects/Windows/tmpl/curl.vcxproj | 48 +++++++++--------- src/tool_doswin.c | 79 ++++++++++++++++++++++++------ 8 files changed, 113 insertions(+), 80 deletions(-) diff --git a/docs/KNOWN_BUGS.md b/docs/KNOWN_BUGS.md index 359c2fab99..59f11de0b5 100644 --- a/docs/KNOWN_BUGS.md +++ b/docs/KNOWN_BUGS.md @@ -105,21 +105,6 @@ would do if you used `-T` file. See [curl issue 12171](https://github.com/curl/curl/issues/12171) -## Windows stdin relay accepts unauthenticated local connections - -curl features a Windows-only stdin relay in `src/tool_doswin.c` that creates a -loopback TCP listener and spawns a thread to accept the first incoming -connection, then forwards stdin to it. There is no authentication or peer -validation on the accepted socket. A local attacker can race to connect to the -ephemeral loopback port (discoverable via local port enumeration/scan) before -curl connects, causing the thread to send stdin/upload data to the attacker or -to disrupt the transfer. - -The function should verify the client-side with a random number similar to the -socketpair emulation function in libcurl. It cannot verify the source address -and port since there is this widespread habit on Windows to run tools that -MITM even local TCP connections for security. - # Build and portability issues ## OS400 port requires deprecated IBM library diff --git a/lib/curlx/winapi.c b/lib/curlx/winapi.c index f025ca48c9..1d6ef87abe 100644 --- a/lib/curlx/winapi.c +++ b/lib/curlx/winapi.c @@ -103,4 +103,24 @@ const char *curlx_winapi_strerror(DWORD err, char *buf, size_t buflen) return buf; } + +#ifndef WITHOUT_LIBCURL + +#include +#ifndef STATUS_SUCCESS +#define STATUS_SUCCESS ((NTSTATUS)0x00000000L) +#endif + +CURLcode curlx_win32_random(unsigned char *entropy, size_t length) +{ + memset(entropy, 0, length); + + if(BCryptGenRandom(NULL, entropy, (ULONG)length, + BCRYPT_USE_SYSTEM_PREFERRED_RNG) != STATUS_SUCCESS) + return CURLE_FAILED_INIT; + + return CURLE_OK; +} +#endif /* WITHOUT_LIBCURL */ + #endif /* _WIN32 */ diff --git a/lib/curlx/winapi.h b/lib/curlx/winapi.h index d30f5efa13..21e864c028 100644 --- a/lib/curlx/winapi.h +++ b/lib/curlx/winapi.h @@ -28,6 +28,7 @@ #define WINAPI_ERROR_LEN 100 const char *curlx_get_winapi_error(DWORD err, char *buf, size_t buflen); const char *curlx_winapi_strerror(DWORD err, char *buf, size_t buflen); +CURLcode curlx_win32_random(unsigned char *entropy, size_t length); #endif #endif /* HEADER_CURLX_WINAPI_H */ diff --git a/lib/rand.c b/lib/rand.c index 3260fe3345..71f629e723 100644 --- a/lib/rand.c +++ b/lib/rand.c @@ -33,23 +33,7 @@ #include "rand.h" #include "escape.h" -#ifdef _WIN32 -#include -#ifndef STATUS_SUCCESS -#define STATUS_SUCCESS ((NTSTATUS)0x00000000L) -#endif - -CURLcode Curl_win32_random(unsigned char *entropy, size_t length) -{ - memset(entropy, 0, length); - - if(BCryptGenRandom(NULL, entropy, (ULONG)length, - BCRYPT_USE_SYSTEM_PREFERRED_RNG) != STATUS_SUCCESS) - return CURLE_FAILED_INIT; - - return CURLE_OK; -} -#endif +#include "curlx/winapi.h" #ifndef USE_SSL /* ---- possibly non-cryptographic version following ---- */ @@ -64,7 +48,7 @@ static CURLcode weak_random(struct Curl_easy *data, #ifdef _WIN32 (void)data; { - CURLcode result = Curl_win32_random(entropy, length); + CURLcode result = curlx_win32_random(entropy, length); if(result != CURLE_NOT_BUILT_IN) return result; } diff --git a/lib/rand.h b/lib/rand.h index a02717074f..452c1b67ef 100644 --- a/lib/rand.h +++ b/lib/rand.h @@ -49,10 +49,4 @@ CURLcode Curl_rand_hex(struct Curl_easy *data, unsigned char *rnd, size_t num); CURLcode Curl_rand_alnum(struct Curl_easy *data, unsigned char *rnd, size_t num); -#ifdef _WIN32 -/* Random generator shared between the Schannel vtls and Curl_rand*() - functions */ -CURLcode Curl_win32_random(unsigned char *entropy, size_t length); -#endif - #endif /* HEADER_CURL_RAND_H */ diff --git a/lib/vtls/schannel.c b/lib/vtls/schannel.c index 6a69237f4e..accda3bbcc 100644 --- a/lib/vtls/schannel.c +++ b/lib/vtls/schannel.c @@ -50,7 +50,7 @@ #include "vtls/x509asn1.h" #include "system_win32.h" #include "curlx/version_win32.h" -#include "rand.h" +#include "curlx/winapi.h" #include "curlx/strparse.h" #include "progress.h" #include "curl_sha256.h" @@ -2637,7 +2637,7 @@ static CURLcode schannel_random(struct Curl_easy *data, { (void)data; - return Curl_win32_random(entropy, length); + return curlx_win32_random(entropy, length); } static CURLcode schannel_checksum(const unsigned char *input, diff --git a/projects/Windows/tmpl/curl.vcxproj b/projects/Windows/tmpl/curl.vcxproj index 6d63601b6b..a6a74e7aad 100644 --- a/projects/Windows/tmpl/curl.vcxproj +++ b/projects/Windows/tmpl/curl.vcxproj @@ -852,7 +852,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win32\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) Console @@ -882,7 +882,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win64\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) Console @@ -911,7 +911,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win32\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) Console @@ -941,7 +941,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win64\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) Console @@ -970,7 +970,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win32\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) true @@ -1002,7 +1002,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win64\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) true @@ -1033,7 +1033,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win32\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) true @@ -1065,7 +1065,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win64\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) true @@ -1096,7 +1096,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win32\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) true @@ -1128,7 +1128,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win64\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) true @@ -1159,7 +1159,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win32\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) true @@ -1190,7 +1190,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win32\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) true @@ -1222,7 +1222,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win64\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) true @@ -1254,7 +1254,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win64\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) true @@ -1285,7 +1285,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win32\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) Console @@ -1315,7 +1315,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win64\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) Console @@ -1344,7 +1344,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win32\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) Console @@ -1373,7 +1373,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win32\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) Console @@ -1403,7 +1403,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win64\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) Console @@ -1433,7 +1433,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win64\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) Console @@ -1462,7 +1462,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win32\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) true @@ -1494,7 +1494,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurld.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win64\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) true @@ -1525,7 +1525,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win32\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) Console @@ -1555,7 +1555,7 @@ ..\..\..\..\include;%(AdditionalIncludeDirectories) - ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) + bcrypt.lib;ws2_32.lib;iphlpapi.lib;wldap32.lib;libcurl.lib;%(AdditionalDependencies) $(OutDir)$(TargetName)$(TargetExt) ..\..\..\..\build\Win64\$SUBDIR\$(Configuration);%(AdditionalLibraryDirectories) Console diff --git a/src/tool_doswin.c b/src/tool_doswin.c index f836af9506..a64665bcd9 100644 --- a/src/tool_doswin.c +++ b/src/tool_doswin.c @@ -29,6 +29,7 @@ #include "curlx/version_win32.h" /* for curlx_verify_windows_version() */ #ifdef _WIN32 +# include "curlx/winapi.h" /* for curlx_win32_random() */ # include #elif !defined(__DJGPP__) || (__DJGPP__ < 2) /* DJGPP 2.0 has _use_lfn() */ # define CURL_USE_LFN(f) 0 /* long filenames never available */ @@ -709,6 +710,9 @@ struct win_thread_data { /* This is the listen socket for the thread. It is closed after the first connection. */ curl_socket_t socket_l; + /* This is the random number which the background thread will use to verify + * the peer. */ + uint64_t expected_auth_val; }; static DWORD WINAPI win_stdin_thread_func(void *thread_data) @@ -716,6 +720,8 @@ static DWORD WINAPI win_stdin_thread_func(void *thread_data) struct win_thread_data *tdata = (struct win_thread_data *)thread_data; struct sockaddr_in clientAddr; int clientAddrLen = sizeof(clientAddr); + size_t nread = 0; + uint64_t auth_val = 0; curl_socket_t socket_w = CURL_ACCEPT(tdata->socket_l, (struct sockaddr *)&clientAddr, @@ -728,6 +734,28 @@ static DWORD WINAPI win_stdin_thread_func(void *thread_data) sclose(tdata->socket_l); tdata->socket_l = CURL_SOCKET_BAD; + + do { + ssize_t ret = sread(socket_w, ((unsigned char *)&auth_val) + nread, + sizeof(auth_val) - nread); + if(ret <= 0) { + if(!ret) { + errorf("relay peer disconnected"); + } + else { + errorf("read error: %d", SOCKERRNO); + } + + goto ThreadCleanup; + } + nread += ret; + } while(nread < sizeof(auth_val)); + + if(auth_val != tdata->expected_auth_val) { + errorf("relay peer auth failed"); + goto ThreadCleanup; + } + if(shutdown(socket_w, SHUT_RD)) { errorf("shutdown error: %d", SOCKERRNO); goto ThreadCleanup; @@ -748,8 +776,6 @@ static DWORD WINAPI win_stdin_thread_func(void *thread_data) break; } ThreadCleanup: - CloseHandle(tdata->stdin_handle); - tdata->stdin_handle = NULL; if(tdata->socket_l != CURL_SOCKET_BAD) { sclose(tdata->socket_l); tdata->socket_l = CURL_SOCKET_BAD; @@ -766,6 +792,9 @@ curl_socket_t win32_stdin_read_thread(void) { int rc = 0; struct win_thread_data *tdata = NULL; + HANDLE stdin_handle = NULL; + uint64_t auth_rnd; + size_t nwritten = 0; static HANDLE stdin_thread = NULL; static curl_socket_t socket_r = CURL_SOCKET_BAD; @@ -815,13 +844,20 @@ curl_socket_t win32_stdin_read_thread(void) break; } + if(curlx_win32_random((unsigned char *)&auth_rnd, sizeof(auth_rnd))) { + errorf("curlx_win32_random() error"); + break; + } + tdata->expected_auth_val = auth_rnd; + /* Make a copy of the stdin handle to be used by win_stdin_thread_func */ if(!DuplicateHandle(GetCurrentProcess(), GetStdHandle(STD_INPUT_HANDLE), - GetCurrentProcess(), &tdata->stdin_handle, + GetCurrentProcess(), &stdin_handle, 0, FALSE, DUPLICATE_SAME_ACCESS)) { errorf("DuplicateHandle error: 0x%08lx", GetLastError()); break; } + tdata->stdin_handle = stdin_handle; /* Start up the thread. We do not bother keeping a reference to it because it runs until program termination. From here on out all reads @@ -850,6 +886,18 @@ curl_socket_t win32_stdin_read_thread(void) break; } + do { + ssize_t ret = swrite(socket_r, ((unsigned char *)&auth_rnd) + nwritten, + sizeof(auth_rnd) - nwritten); + + if(ret <= 0) { + errorf("socket write error: %d", SOCKERRNO); + goto err; + } + + nwritten += ret; + } while(nwritten < sizeof(auth_rnd)); + if(shutdown(socket_r, SHUT_WR)) { errorf("shutdown error: %d", SOCKERRNO); break; @@ -864,25 +912,26 @@ curl_socket_t win32_stdin_read_thread(void) rc = 1; } while(0); +err: if(rc != 1) { - if(socket_r != CURL_SOCKET_BAD && tdata) { - if(GetStdHandle(STD_INPUT_HANDLE) == (HANDLE)socket_r && - tdata->stdin_handle) { - /* restore STDIN */ - SetStdHandle(STD_INPUT_HANDLE, tdata->stdin_handle); - tdata->stdin_handle = NULL; - } - - sclose(socket_r); - socket_r = CURL_SOCKET_BAD; - } - if(stdin_thread) { TerminateThread(stdin_thread, 1); CloseHandle(stdin_thread); stdin_thread = NULL; } + if(socket_r != CURL_SOCKET_BAD) { + if(GetStdHandle(STD_INPUT_HANDLE) == (HANDLE)socket_r && + stdin_handle) { + /* restore STDIN */ + SetStdHandle(STD_INPUT_HANDLE, stdin_handle); + stdin_handle = NULL; + } + + sclose(socket_r); + socket_r = CURL_SOCKET_BAD; + } + if(tdata) { if(tdata->stdin_handle) CloseHandle(tdata->stdin_handle);