[WS2_32]: More fixes:
authorHermès Bélusca-Maïto <hermes.belusca-maito@reactos.org>
Sat, 4 Mar 2017 00:35:02 +0000 (00:35 +0000)
committerHermès Bélusca-Maïto <hermes.belusca-maito@reactos.org>
Sat, 4 Mar 2017 00:35:02 +0000 (00:35 +0000)
- 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
reactos/dll/win32/ws2_32/src/nsquery.c
reactos/dll/win32/ws2_32/src/qshelpr.c
reactos/dll/win32/ws2_32/src/rnr.c
reactos/dll/win32/ws2_32/src/wsautil.c

index 4b3cdd4..df353ad 100644 (file)
@@ -847,6 +847,8 @@ WsAsyncCheckAndInitThread(VOID)
     {
         /* Initialize Thread Context */
         Context = HeapAlloc(WsSockHeap, 0, sizeof(*Context));
     {
         /* Initialize Thread Context */
         Context = HeapAlloc(WsSockHeap, 0, sizeof(*Context));
+        if (!Context)
+            goto Exit;
 
         /* Initialize the Queue and event */
         WsAsyncQueue = &Context->AsyncQueue;
 
         /* 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 */
         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,
 
         /* Create the thread */
         ThreadHandle = CreateThread(NULL,
@@ -864,15 +867,31 @@ WsAsyncCheckAndInitThread(VOID)
                                     Context,
                                     0,
                                     &Tid);
                                     Context,
                                     0,
                                     &Tid);
+        if (ThreadHandle == NULL)
+        {
+            /* Cleanup and fail */
+            WSACleanup();
+            goto Fail;
+        }
 
         /* Close the handle and set init */
         CloseHandle(ThreadHandle);
         WsAsyncThreadInitialized = TRUE;
     }
 
 
         /* Close the handle and set init */
         CloseHandle(ThreadHandle);
         WsAsyncThreadInitialized = TRUE;
     }
 
+Exit:
     /* Release the lock */
     WsAsyncUnlock();
     return WsAsyncThreadInitialized;
     /* 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
 }
 
 VOID
index a361819..c56066c 100644 (file)
@@ -268,15 +268,12 @@ WsNqLookupServiceNext(IN PNSQUERY NsQuery,
         /* Acquire Query Lock */
         WsNqLock();
 
         /* 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)
 
             /* 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 =
                 {
                     /* New query succeeded, set active provider now */
                     NsQuery->ActiveProvider =
-                        WsNqNextProvider(NsQuery,
-                                         NsQuery->ActiveProvider);
+                        WsNqNextProvider(NsQuery, NsQuery->ActiveProvider);
                 }
             }
             else
                 }
             }
             else
index 43dd6b6..9354c69 100644 (file)
@@ -499,8 +499,8 @@ CopyQuerySetIndirectA(IN PWS_BUFFER Buffer,
                                                     sizeof(PVOID));
 
         /* Copy it into the 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 */
                       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 */
                                                     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 */
                       UnicodeSet->dwNumberOfCsAddrs * sizeof(CSADDR_INFO));
 
         /* Copy the addresses inside the CSADDR */
index 53d4631..579b85a 100644 (file)
@@ -397,8 +397,9 @@ WSALookupServiceNextW(IN HANDLE hLookup,
         return SOCKET_ERROR;
     }
 
         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);
     {
         /* It is invalid; fail */
         SetLastError(WSAEFAULT);
@@ -437,10 +438,21 @@ WSALookupServiceNextA(IN HANDLE hLookup,
                       OUT LPWSAQUERYSETA lpqsResults)
 {
     LPWSAQUERYSETW UnicodeQuerySet;
                       OUT LPWSAQUERYSETA lpqsResults)
 {
     LPWSAQUERYSETW UnicodeQuerySet;
-    DWORD UnicodeQuerySetSize = *lpdwBufferLength;
+    DWORD UnicodeQuerySetSize;
     INT ErrorCode;
     DPRINT("WSALookupServiceNextA: %lx\n", hLookup);
 
     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))
     {
     /* Check how much the user is giving */
     if (UnicodeQuerySetSize >= sizeof(WSAQUERYSETW))
     {
index 9c89489..d40ea28 100644 (file)
@@ -31,15 +31,15 @@ WsOpenRegistryRoot(VOID)
     if (ErrorCode == ERROR_FILE_NOT_FOUND)
     {
         /* Create it */
     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)
     {
     }
     else if (ErrorCode == ERROR_SUCCESS)
     {