From 99e8525cb3228ac33dc5d15584bcc765a606f682 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herm=C3=A8s=20B=C3=A9lusca-Ma=C3=AFto?= Date: Sat, 4 Mar 2017 00:35:02 +0000 Subject: [PATCH] [WS2_32]: More fixes: - Perform success checks in WsAsyncCheckAndInitThread, in particular, check whether Context is correctly allocated, and check whether the WsAsyncThread was correctly started up. In case of failure, perform the necessary cleanup, including calling WSACleanup(). - Check also the returned error code of WSAStartup. Fixes CID 1101934. - Fix logic mess-up in WsNqLookupServiceNext when updating NsQuery->ActiveProvider; - Fix copy-pasta errors (using 'lpafpProtocols' instead of 'lpcsaBuffer') in CopyQuerySetIndirectA and CopyQuerySetIndirectW, that triggered CID 513446 + CID 513447 (CopyQuerySetIndirectA), and CID 513444 + CID 513445 (CopyQuerySetIndirectW). - Check for 'lpdwBufferLength' pointer validity in WSALookupServiceNextW; - Check for 'lpdwBufferLength' and 'lpqsResults' pointers validity in WSALookupServiceNextA, and dereference lpdwBufferLength only afterwards. - Check for return value of RegCreateKeyEx in WsOpenRegistryRoot(), fixes CID 715923. svn path=/trunk/; revision=74044 --- reactos/dll/win32/ws2_32/src/async.c | 21 ++++++++++++++++++++- reactos/dll/win32/ws2_32/src/nsquery.c | 16 ++++++---------- reactos/dll/win32/ws2_32/src/qshelpr.c | 8 ++++---- reactos/dll/win32/ws2_32/src/rnr.c | 18 +++++++++++++++--- reactos/dll/win32/ws2_32/src/wsautil.c | 18 +++++++++--------- 5 files changed, 54 insertions(+), 27 deletions(-) diff --git a/reactos/dll/win32/ws2_32/src/async.c b/reactos/dll/win32/ws2_32/src/async.c index 4b3cdd4f257..df353adf117 100644 --- a/reactos/dll/win32/ws2_32/src/async.c +++ b/reactos/dll/win32/ws2_32/src/async.c @@ -847,6 +847,8 @@ WsAsyncCheckAndInitThread(VOID) { /* Initialize Thread Context */ Context = HeapAlloc(WsSockHeap, 0, sizeof(*Context)); + if (!Context) + goto Exit; /* Initialize the Queue and event */ WsAsyncQueue = &Context->AsyncQueue; @@ -855,7 +857,8 @@ WsAsyncCheckAndInitThread(VOID) WsAsyncEvent = Context->AsyncEvent; /* Prevent us from ever being killed while running */ - WSAStartup(MAKEWORD(2,2), &WsaData); + if (WSAStartup(MAKEWORD(2,2), &WsaData) != ERROR_SUCCESS) + goto Fail; /* Create the thread */ ThreadHandle = CreateThread(NULL, @@ -864,15 +867,31 @@ WsAsyncCheckAndInitThread(VOID) Context, 0, &Tid); + if (ThreadHandle == NULL) + { + /* Cleanup and fail */ + WSACleanup(); + goto Fail; + } /* Close the handle and set init */ CloseHandle(ThreadHandle); WsAsyncThreadInitialized = TRUE; } +Exit: /* Release the lock */ WsAsyncUnlock(); return WsAsyncThreadInitialized; + +Fail: + /* Close the event, free the Context */ + if (Context->AsyncEvent) + CloseHandle(Context->AsyncEvent); + HeapFree(WsSockHeap, 0, Context); + + /* Bail out */ + goto Exit; } VOID diff --git a/reactos/dll/win32/ws2_32/src/nsquery.c b/reactos/dll/win32/ws2_32/src/nsquery.c index a361819e362..c56066c527c 100644 --- a/reactos/dll/win32/ws2_32/src/nsquery.c +++ b/reactos/dll/win32/ws2_32/src/nsquery.c @@ -268,15 +268,12 @@ WsNqLookupServiceNext(IN PNSQUERY NsQuery, /* Acquire Query Lock */ WsNqLock(); - /* Save the current active provider */ - Provider = NsQuery->ActiveProvider; - - /* Check if one exists */ - if (Provider) + /* Check if we have an active provider */ + if (NsQuery->ActiveProvider) { - /* Get the next one */ - NextProvider = WsNqNextProvider(NsQuery, - NsQuery->ActiveProvider); + /* Save the old provider and get the next one */ + Provider = NextProvider; + NextProvider = WsNqNextProvider(NsQuery, NsQuery->ActiveProvider); /* Was the old provider our active? */ if (Provider == NsQuery->ActiveProvider) @@ -327,8 +324,7 @@ WsNqLookupServiceNext(IN PNSQUERY NsQuery, { /* New query succeeded, set active provider now */ NsQuery->ActiveProvider = - WsNqNextProvider(NsQuery, - NsQuery->ActiveProvider); + WsNqNextProvider(NsQuery, NsQuery->ActiveProvider); } } else diff --git a/reactos/dll/win32/ws2_32/src/qshelpr.c b/reactos/dll/win32/ws2_32/src/qshelpr.c index 43dd6b62a41..9354c69ba93 100644 --- a/reactos/dll/win32/ws2_32/src/qshelpr.c +++ b/reactos/dll/win32/ws2_32/src/qshelpr.c @@ -499,8 +499,8 @@ CopyQuerySetIndirectA(IN PWS_BUFFER Buffer, sizeof(PVOID)); /* Copy it into the buffer */ - RtlCopyMemory(RelativeSet->lpafpProtocols, - AnsiSet->lpafpProtocols, + RtlCopyMemory(RelativeSet->lpcsaBuffer, + AnsiSet->lpcsaBuffer, AnsiSet->dwNumberOfCsAddrs * sizeof(CSADDR_INFO)); /* Copy the addresses inside the CSADDR */ @@ -693,8 +693,8 @@ CopyQuerySetIndirectW(IN PWS_BUFFER Buffer, sizeof(PVOID)); /* Copy it into the buffer */ - RtlCopyMemory(RelativeSet->lpafpProtocols, - UnicodeSet->lpafpProtocols, + RtlCopyMemory(RelativeSet->lpcsaBuffer, + UnicodeSet->lpcsaBuffer, UnicodeSet->dwNumberOfCsAddrs * sizeof(CSADDR_INFO)); /* Copy the addresses inside the CSADDR */ diff --git a/reactos/dll/win32/ws2_32/src/rnr.c b/reactos/dll/win32/ws2_32/src/rnr.c index 53d4631a8e7..579b85ab39a 100644 --- a/reactos/dll/win32/ws2_32/src/rnr.c +++ b/reactos/dll/win32/ws2_32/src/rnr.c @@ -397,8 +397,9 @@ WSALookupServiceNextW(IN HANDLE hLookup, return SOCKET_ERROR; } - /* Verify pointer */ - if (IsBadWritePtr(lpqsResults, sizeof(*lpqsResults))) + /* Verify pointers */ + if (IsBadReadPtr(lpdwBufferLength, sizeof(*lpdwBufferLength)) || + IsBadWritePtr(lpqsResults, sizeof(*lpqsResults))) { /* It is invalid; fail */ SetLastError(WSAEFAULT); @@ -437,10 +438,21 @@ WSALookupServiceNextA(IN HANDLE hLookup, OUT LPWSAQUERYSETA lpqsResults) { LPWSAQUERYSETW UnicodeQuerySet; - DWORD UnicodeQuerySetSize = *lpdwBufferLength; + DWORD UnicodeQuerySetSize; INT ErrorCode; DPRINT("WSALookupServiceNextA: %lx\n", hLookup); + /* Verify pointers */ + if (IsBadReadPtr(lpdwBufferLength, sizeof(*lpdwBufferLength)) || + IsBadWritePtr(lpqsResults, sizeof(*lpqsResults))) + { + /* It is invalid; fail */ + SetLastError(WSAEFAULT); + return SOCKET_ERROR; + } + + UnicodeQuerySetSize = *lpdwBufferLength; + /* Check how much the user is giving */ if (UnicodeQuerySetSize >= sizeof(WSAQUERYSETW)) { diff --git a/reactos/dll/win32/ws2_32/src/wsautil.c b/reactos/dll/win32/ws2_32/src/wsautil.c index 9c894895ad0..d40ea2829b5 100644 --- a/reactos/dll/win32/ws2_32/src/wsautil.c +++ b/reactos/dll/win32/ws2_32/src/wsautil.c @@ -31,15 +31,15 @@ WsOpenRegistryRoot(VOID) if (ErrorCode == ERROR_FILE_NOT_FOUND) { /* Create it */ - RegCreateKeyEx(HKEY_LOCAL_MACHINE, - WINSOCK_ROOT, - 0, - NULL, - REG_OPTION_NON_VOLATILE, - KEY_ALL_ACCESS, - NULL, - &WinsockRootKey, - &CreateDisposition); + ErrorCode = RegCreateKeyEx(HKEY_LOCAL_MACHINE, + WINSOCK_ROOT, + 0, + NULL, + REG_OPTION_NON_VOLATILE, + KEY_ALL_ACCESS, + NULL, + &WinsockRootKey, + &CreateDisposition); } else if (ErrorCode == ERROR_SUCCESS) {