[389-commits] 5 commits - admserv/cgi-ds admserv/cgi-src40 lib/libdsa

Nathan Kinder nkinder at fedoraproject.org
Fri May 6 21:16:05 UTC 2011


 admserv/cgi-ds/ds_snmpctrl.c  |    3 +-
 admserv/cgi-src40/htmladmin.c |   59 ++++++++++++++++++++++++++++++++----------
 admserv/cgi-src40/security.c  |    6 ++++
 admserv/cgi-src40/viewlog.c   |   31 +++++++++++++++++-----
 lib/libdsa/dsalib_location.c  |    2 -
 lib/libdsa/dsalib_util.c      |    5 ++-
 6 files changed, 82 insertions(+), 24 deletions(-)

New commits:
commit 130ea2ff6c64cefd0f5d7c7aedd9e22411fb0e96
Author: Nathan Kinder <nkinder at redhat.com>
Date:   Fri May 6 13:51:44 2011 -0700

    Bug 702705 - (cov#10784,10783) NULL pointer dereferences in dsalib
    
    There are a few variables in dsalib that we need to check for NULL
    before dereferencing.

diff --git a/lib/libdsa/dsalib_location.c b/lib/libdsa/dsalib_location.c
index 1ed29ea..ef2dd5e 100644
--- a/lib/libdsa/dsalib_location.c
+++ b/lib/libdsa/dsalib_location.c
@@ -278,7 +278,7 @@ ds_get_server_name()
         buf = getenv("SCRIPT_NAME");
         if ( buf && (*buf == '/') )
             buf++;
-        while ( *buf && (*buf != '/') ) {
+        while ( buf && *buf && (*buf != '/') ) {
             *out++ = *buf++;
         }
         *out = 0;
diff --git a/lib/libdsa/dsalib_util.c b/lib/libdsa/dsalib_util.c
index 8f2fc8f..a03c827 100644
--- a/lib/libdsa/dsalib_util.c
+++ b/lib/libdsa/dsalib_util.c
@@ -121,7 +121,10 @@ ds_get_file_list( char *dir )
     }
     (void) closedir( dirp );
 
-    ret[ nfiles ] = NULL;
+    if (ret) {
+        ret[ nfiles ] = NULL;
+    }
+
     return ret;
 }
 #else


commit 8e7ac252eef0c87d55e71da33b3c557ae844822f
Author: Nathan Kinder <nkinder at redhat.com>
Date:   Fri May 6 13:45:39 2011 -0700

    Bug 702705 - (cov#10785) NULL pointer dereference in ds_snmpctrl
    
    We should only call fclose() if fopen() was successful.

diff --git a/admserv/cgi-ds/ds_snmpctrl.c b/admserv/cgi-ds/ds_snmpctrl.c
index 757d60a..797ea9d 100644
--- a/admserv/cgi-ds/ds_snmpctrl.c
+++ b/admserv/cgi-ds/ds_snmpctrl.c
@@ -121,9 +121,10 @@ get_nsldapagt_pid(pid_t *pid)
 	    (void) fclose(fp);
 	    return(0);
       }
+
+      (void) fclose(fp);
    }
 
-   (void) fclose(fp);
    return(-1);
 }
 #endif


commit 2b66b5706f1805f963eb6bb53afad5e5197cf152
Author: Nathan Kinder <nkinder at redhat.com>
Date:   Fri May 6 11:59:48 2011 -0700

    Bug 702705 - (cov#10803) NULL pointer dereference in security cgi
    
    There is a chance we can deference a NULL variable if the format
    CGI variable ends up being NULL.  This is unlikely, since the CGI
    will exit when there is a problem fetching the variable, but we
    should check for NULL to be safe.

diff --git a/admserv/cgi-src40/security.c b/admserv/cgi-src40/security.c
index b8808a1..ce84a81 100644
--- a/admserv/cgi-src40/security.c
+++ b/admserv/cgi-src40/security.c
@@ -1889,6 +1889,12 @@ static void moduleOperation(char* op) {
       rpt_err(FILE_ERROR, msg, NULL, NULL);
     }
 
+    /* Bail if the format wasn't specified. */
+    if (!filetype) {
+        PR_snprintf(msg, sizeof(msg), getResourceString(DBT_MISSING_FORMAT));
+        rpt_err(INCORRECT_USAGE, msg, NULL, NULL);
+    }
+
     /* dllname is the internal name of the module - the user must
        have specified a valid name */
     if((filetype && (!PORT_Strcmp(filetype, "dll"))) &&


commit ee0af5321650e5da56461050d60b83ba86f5d18d
Author: Nathan Kinder <nkinder at redhat.com>
Date:   Fri May 6 10:25:58 2011 -0700

    Bug 702705 - NULL pointer dereferences in viewlog cgi
    
    A cut and paste error was made where we check if we have a valid
    filename.  The logical ORs need to be changed to ANDs to avoid the
    NULL pointer dereference. (cov#10811)
    
    The num integer needs to be checked for a negative value to prevent
    calling free() on a NULL pointer.  I also added checking for malloc()
    and realloc() errors so we can bail out before dereferencing a NULL
    pointer. (cov#10809)

diff --git a/admserv/cgi-src40/viewlog.c b/admserv/cgi-src40/viewlog.c
index a24130b..bda9338 100644
--- a/admserv/cgi-src40/viewlog.c
+++ b/admserv/cgi-src40/viewlog.c
@@ -105,8 +105,10 @@ void search_file(FILE *cmd, int num, char *str) {
   int replace, count, temp;
   char line[BIG_LINE];
   char **buffer = NULL; /* buffer to store log matches */
+  char **tmp = NULL;
+  int i = 0;
 
-  if(num == 0) return;
+  if(num <= 0) return;
 
   replace=0;
   count=0;
@@ -115,13 +117,28 @@ void search_file(FILE *cmd, int num, char *str) {
       if((!str) || (strstr(line, str))) {
         count++;
         if(count<=num) {
-          if(!buffer)
+          if(!buffer) {
             buffer = (char **)MALLOC(sizeof(char *));
-          else
-            buffer = (char **)REALLOC(buffer, count*(sizeof(char *)));
-        }
-        else
+            if (!buffer) {
+              /* Malloc failed.  Bail. */
+              return;
+            }
+          } else {
+            tmp = (char **)REALLOC(buffer, count*(sizeof(char *)));
+            if (!tmp) {
+              /* Realloc failed.  Free and bail. */
+              for (i=0; i<(count-1); i++) {
+                FREE(buffer[i]);
+              }
+              FREE(buffer);
+              return;
+            }
+            buffer = tmp;
+          }
+        } else {
           FREE(buffer[replace]);
+        }
+
         temp = strlen(line);
         line[temp-1] = ' ';
         line[temp] = '\n';
@@ -464,7 +481,7 @@ int main(int argc, char *argv[])
 #ifdef AIX
         fflush(stdout);
 #endif
-        if (file || *file || util_is_valid_path_string(file) ||
+        if (file && *file && util_is_valid_path_string(file) && 
             util_verify_file_or_dir(logdir, PR_FILE_DIRECTORY, file, -1, PR_FILE_FILE)) {
             PR_snprintf(full_path, sizeof(full_path), "%s%c%s", logdir, FILE_PATHSEP, file);
             cmd = fopen(full_path, "r");


commit c82c134a17831d4b6bc724b561028e7fb26e07e8
Author: Nathan Kinder <nkinder at redhat.com>
Date:   Fri May 6 09:29:24 2011 -0700

    Bug 702705 - (cov#10830) NULL pointer dereference in htmladmin
    
    If malloc() or realloc() fails, we could end up dereferencing a NULL
    pointer. We should check to see if memory was successfully allocated
    before dereferencing it.

diff --git a/admserv/cgi-src40/htmladmin.c b/admserv/cgi-src40/htmladmin.c
index 4ef1229..e79ad82 100644
--- a/admserv/cgi-src40/htmladmin.c
+++ b/admserv/cgi-src40/htmladmin.c
@@ -596,7 +596,9 @@ char **get_all_users_views(LDAP *server, char *binddn, AdmldapInfo ldapInfo) {
   LDAPMessage *result;
 
   char **return_array = NULL;
+  char **tmp = NULL;
   int i;
+  int error = 0;
   char *escaped_binddn = NULL;
 
   if(!binddn)
@@ -633,9 +635,11 @@ char **get_all_users_views(LDAP *server, char *binddn, AdmldapInfo ldapInfo) {
                                 filter, NULL, 0,
                                 NULL, NULL, NULL, -1, &result);
 
-  if(ldapError != LDAP_SUCCESS)
+  if(ldapError != LDAP_SUCCESS) {
     /* fatal error, bail */
-    return NULL;
+    error = 1;
+    goto bail;
+  }
 
   i=0;
   for(entry = ldap_first_entry(server, result);
@@ -649,15 +653,28 @@ char **get_all_users_views(LDAP *server, char *binddn, AdmldapInfo ldapInfo) {
     
     if(!return_array) {
       return_array = (char **)malloc(sizeof(char *));
+      if (!return_array) {
+        error = 1;
+        goto bail;
+      }
       return_array[0] = strdup(vals[0]);
-    }
-    else {
-      return_array = (char **)realloc(return_array, (i+1)*sizeof(char *));
+    } else {
+      tmp = (char **)realloc(return_array, (i+1)*sizeof(char *));
+      if (!tmp) {
+        error = 1;
+        goto bail;
+      }
+      return_array = tmp;
       return_array[i] = strdup(vals[0]);
     }
     i++;
   }
-  return_array = (char **)realloc(return_array, (i+1)*sizeof(char *));
+  tmp = (char **)realloc(return_array, (i+1)*sizeof(char *));
+  if (!tmp) {
+    error = 1;
+    goto bail;
+  }
+  return_array = tmp;
   return_array[i] = NULL;
 
   /* Next, search public views */
@@ -684,19 +701,33 @@ char **get_all_users_views(LDAP *server, char *binddn, AdmldapInfo ldapInfo) {
     if(!vals || !vals[0])
       break;
     
-    if(!return_array) {
-      return_array = (char **)malloc(sizeof(char *));
-      return_array[0] = strdup(vals[0]);
-    }
-    else {
-      return_array = (char **)realloc(return_array, (i+1)*sizeof(char *));
-      return_array[i] = strdup(vals[0]);
+    tmp = (char **)realloc(return_array, (i+1)*sizeof(char *));
+    if (!tmp) {
+      error = 1;
+      goto bail;
     }
+    return_array = tmp;
+    return_array[i] = strdup(vals[0]);
     i++;
   }
-  return_array = (char **)realloc(return_array, (i+1)*sizeof(char *));
+  tmp = (char **)realloc(return_array, (i+1)*sizeof(char *));
+  if (!tmp) {
+    error = 1;
+    goto bail;
+  }
+  return_array = tmp;
   return_array[i] = NULL;
 
+bail:
+  if (error) {
+    /* An error occurred, so free the array. */
+    for (i=0; return_array && return_array[i]; i++) {
+      free((void *)return_array[i]);
+    }
+    free((void *)return_array);
+    return_array = NULL;
+  }
+
   return return_array;
 }
 




More information about the 389-commits mailing list