[ros-diffs] [cgutman] 42774: - Fix null pointer dereferences - Fix out of bounds array access - Check that we got a valid pointer from HeapAlloc - Don't close the caller's handle when we fail - Fix a memory leak - Found by Amine Khaldi

Author: cgutman
Date: Tue Aug 18 22:30:58 2009
New Revision: 42774

URL: http://svn.reactos.org/svn/reactos?rev=42774&view=rev
Log:
 - Fix null pointer dereferences
 - Fix out of bounds array access
 - Check that we got a valid pointer from HeapAlloc
 - Don't close the caller's handle when we fail
 - Fix a memory leak
 - Found by Amine Khaldi

Modified:
    trunk/reactos/dll/win32/iphlpapi/ifenum_reactos.c
    trunk/reactos/dll/win32/iphlpapi/iphlpapi_main.c
    trunk/reactos/dll/win32/iphlpapi/ipstats_reactos.c
    trunk/reactos/dll/win32/iphlpapi/registry.c
    trunk/reactos/dll/win32/iphlpapi/resinfo_reactos.c

Modified: trunk/reactos/dll/win32/iphlpapi/ifenum_reactos.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/iphlpapi/ifenum_…
==============================================================================
--- trunk/reactos/dll/win32/iphlpapi/ifenum_reactos.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/iphlpapi/ifenum_reactos.c [iso-8859-1] Tue Aug 18 22:30:58
2009
@@ -582,6 +582,8 @@
 
             interfaceName = HeapAlloc( GetProcessHeap(), 0,
                                        strlen(adapter_name) + 1 );
+            if (!interfaceName) return NULL;
+
             strcpy( interfaceName, adapter_name );
         }
 
@@ -847,15 +849,14 @@
 
 char *toIPAddressString(unsigned int addr, char string[16])
 {
-  if (string) {
     struct in_addr iAddr;
 
     iAddr.s_addr = addr;
-    /* extra-anal, just to make auditors happy */
-    strncpy(string, inet_ntoa(iAddr), 16);
-    string[16] = '\0';
-  }
-  return string;
+
+    if (string)
+        strncpy(string, inet_ntoa(iAddr), 16);
+  
+    return inet_ntoa(iAddr);
 }
 
 NTSTATUS addIPAddress( IPAddr Address, IPMask Mask, DWORD IfIndex,

Modified: trunk/reactos/dll/win32/iphlpapi/iphlpapi_main.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/iphlpapi/iphlpap…
==============================================================================
--- trunk/reactos/dll/win32/iphlpapi/iphlpapi_main.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/iphlpapi/iphlpapi_main.c [iso-8859-1] Tue Aug 18 22:30:58
2009
@@ -102,6 +102,7 @@
 {
    DWORD ndx, retVal = 0, numRoutes = getNumRoutes();
    RouteTable *table = getRouteTable();
+   if (!table) return 0;
 
     for (ndx = 0; ndx < numRoutes; ndx++)
     {
@@ -645,9 +646,12 @@
               DWORD addrLen = sizeof(ptr->Address), type;
               const char *ifname =
                   getInterfaceNameByIndex(table->indexes[ndx]);
+              if (!ifname) {
+                  ret = ERROR_OUTOFMEMORY;
+                  break;
+              }
 
               /* on Win98 this is left empty, but whatever */
-
               strncpy(ptr->AdapterName,ifname,sizeof(ptr->AdapterName));
               consumeInterfaceName(ifname);
               ptr->AdapterName[MAX_ADAPTER_NAME_LENGTH] = '\0';
@@ -983,9 +987,9 @@
     }
     else {
       InterfaceIndexTable *table = getNonLoopbackInterfaceIndexTable();
-      TRACE("table->numIndexes == 0x%x\n", table->numIndexes);
 
       if (table) {
+        TRACE("table->numIndexes == 0x%x\n", table->numIndexes);
         size = sizeof(IP_INTERFACE_INFO) + (table->numIndexes) *
          sizeof(IP_ADAPTER_INDEX_MAP);
         if (*dwOutBufLen < size) {

Modified: trunk/reactos/dll/win32/iphlpapi/ipstats_reactos.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/iphlpapi/ipstats…
==============================================================================
--- trunk/reactos/dll/win32/iphlpapi/ipstats_reactos.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/iphlpapi/ipstats_reactos.c [iso-8859-1] Tue Aug 18 22:30:58
2009
@@ -459,6 +459,10 @@
     out_route_table = HeapAlloc( GetProcessHeap(), 0,
                                  sizeof(RouteTable) +
                                  (sizeof(RouteEntry) * (numRoutes - 1)) );
+    if (!out_route_table) {
+        closeTcpFile(tcpFile);
+        return NULL;
+    }
 
     out_route_table->numRoutes = numRoutes;
 
@@ -586,6 +590,10 @@
     IpArpTable = HeapAlloc
 	( GetProcessHeap(), 0,
 	  sizeof(DWORD) + (sizeof(MIB_IPNETROW) * totalNumber) );
+    if (!IpArpTable) {
+        closeTcpFile(tcpFile);
+        return NULL;
+    }
 
     status = tdiGetEntityIDSet( tcpFile, &entitySet, &numEntities );
 

Modified: trunk/reactos/dll/win32/iphlpapi/registry.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/iphlpapi/registr…
==============================================================================
--- trunk/reactos/dll/win32/iphlpapi/registry.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/iphlpapi/registry.c [iso-8859-1] Tue Aug 18 22:30:58 2009
@@ -44,18 +44,19 @@
   PWCHAR Value;
   DWORD ValueLen;
 
-  if (MaxAdapterName == -1) {
-    RegCloseKey( RegHandle );
+  if (MaxAdapterName == -1)
     return 0;
-  }
 
   ValueLen = MaxAdapterName;
   Value = (PWCHAR)HeapAlloc( GetProcessHeap(), 0, MaxAdapterName * sizeof(WCHAR) );
+  if (!Value) return 0;
+
   Status = RegEnumKeyExW( RegHandle, n, Value, &ValueLen,
 			  NULL, NULL, NULL, NULL );
-  if (Status != ERROR_SUCCESS)
+  if (Status != ERROR_SUCCESS) {
+    HeapFree(GetProcessHeap(), 0, Value);
     return 0;
-  else {
+  } else {
     Value[ValueLen] = 0;
     return Value;
   }

Modified: trunk/reactos/dll/win32/iphlpapi/resinfo_reactos.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/iphlpapi/resinfo…
==============================================================================
--- trunk/reactos/dll/win32/iphlpapi/resinfo_reactos.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/iphlpapi/resinfo_reactos.c [iso-8859-1] Tue Aug 18 22:30:58
2009
@@ -151,11 +151,13 @@
 	}
 	if (ch - LastNameStart > 0) { /* A last name? */
 	    PWCHAR NameServer = malloc(((ch - LastNameStart) + 1) * sizeof(WCHAR));
-	    memcpy(NameServer,NameServerString + LastNameStart,
-		   (ch - LastNameStart) * sizeof(WCHAR));
-	    NameServer[ch - LastNameStart] = 0;
-	    cb( Interface, NameServer, Data );
-	    free(NameServer);
+            if (NameServer) {
+	        memcpy(NameServer,NameServerString + LastNameStart,
+		       (ch - LastNameStart) * sizeof(WCHAR));
+	        NameServer[ch - LastNameStart] = 0;
+	        cb( Interface, NameServer, Data );
+	        free(NameServer);
+            }
 	}
 	ConsumeRegValueString(NameServerString);
     }
@@ -223,6 +225,8 @@
 
     PrivateNSEnum.NumServers = ServerCount;
     DnsList = HeapAlloc(GetProcessHeap(), 0, ServerCount * sizeof(IP_ADDR_STRING));
+    if (!DnsList) return NULL;
+
     ZeroMemory(DnsList, ServerCount * sizeof(IP_ADDR_STRING));
 
     ResInfo = (PIPHLP_RES_INFO)RtlAllocateHeap ( GetProcessHeap(), 0,
sizeof(IPHLP_RES_INFO));