[prev in list] [next in list] [prev in thread] [next in thread] 

List:       fedora-directory-commits
Subject:    [389-commits] 7 commits - admserv/cgi-ds admserv/cgi-src40
From:       Nathan Kinder <nkinder () fedoraproject ! org>
Date:       2011-05-05 20:56:42
Message-ID: 20110505205642.F3713120213 () lists ! fedorahosted ! org
[Download RAW message or body]

 admserv/cgi-ds/ds_listdb.c    |    5 
 admserv/cgi-src40/ReadLog.c   |   18 -
 admserv/cgi-src40/config.c    |   13 -
 admserv/cgi-src40/help.c      |    4 
 admserv/cgi-src40/htmladmin.c |  450 +++++++++++++++++-------------------------
 include/libdsa/dsalib.h       |    1 
 lib/libdsa/dsalib_conf.c      |   17 +
 lib/libdsa/dsalib_confs.c     |    5 
 lib/libdsa/dsalib_location.c  |    3 
 lib/libdsa/dsalib_tailf.c     |    1 
 lib/libdsa/dsalib_util.c      |    1 
 11 files changed, 245 insertions(+), 273 deletions(-)

New commits:
commit b427aa1eb706b269c2948355625fd6998917941a
Author: Nathan Kinder <nkinder@redhat.com>
Date:   Thu May 5 13:55:45 2011 -0700

    Bug 702150 - Resouce leaks in htmladmin.c
    
    There are a number of places where htmladmin_strdup_escaped() is
    called without saving the returned pointer.  This doesn't allow us
    to ever free the returned string.  These areas have been changed to
    save and free the returned value.  Most of these places were using
    malloc() and then sprintf().  I have changed all of these to use
    PR_smprintf() to prevent errors in allocating the wrong amount of
    memory.
    
    The start_server() and stop_server() functions also allocated a few
    strings without freeing them when finished.  I added bail sections
    at the end of these functions that free the memory.
    
    A few functions were leaking dynamic arrays.  I added deep frees of
    the arrays and their elements prior to exiting the functions.
    
    cov#10829,10828,10827,10826,10825,10824

diff --git a/admserv/cgi-src40/htmladmin.c b/admserv/cgi-src40/htmladmin.c
index 8ed69bf..4ef1229 100644
--- a/admserv/cgi-src40/htmladmin.c
+++ b/admserv/cgi-src40/htmladmin.c
@@ -666,9 +666,14 @@ char **get_all_users_views(LDAP *server, char *binddn, \
AdmldapInfo ldapInfo) {  ldapError = ldap_search_ext_s(server, dn, \
LDAP_SCOPE_SUBTREE,  filter, NULL, 0,
                                 NULL, NULL, NULL, -1, &result);
-  if((ldapError != LDAP_SUCCESS) && (ldapError != LDAP_NO_SUCH_OBJECT))
-    /* fatal error, bail */
+  if((ldapError != LDAP_SUCCESS) && (ldapError != LDAP_NO_SUCH_OBJECT)) {
+    /* fatal error, free array, bail */
+    for (i=0; return_array && return_array[i]; i++) {
+      free((void *)return_array[i]);
+    }
+    free((void *)return_array);
     return NULL;
+  }
 
   for(entry = ldap_first_entry(server, result);
       entry != NULL;
@@ -919,6 +924,8 @@ int output_topology(AdmldapInfo ldapInfo,
   char **view_list;
   int first_servergroup;
   int legacy;
+  int i = 0;
+  int rc = 0;
 
   server = server_bind(securitydir, host, port, security, binddn, bindpw);
   PL_strfree(securitydir);
@@ -937,8 +944,10 @@ int output_topology(AdmldapInfo ldapInfo,
   /* DOMAIN */
 
   if((ldapError = sorted_search(DOMAIN_ATTR, server, NETSCAPE_ROOT, \
                LDAP_SCOPE_ONELEVEL,
-				DOMAIN_OBJTYPE, NULL, 0, &domain_result)) != LDAP_SUCCESS)
-    return -1;
+				DOMAIN_OBJTYPE, NULL, 0, &domain_result)) != LDAP_SUCCESS) {
+    rc = -1;
+    goto bail;
+  }
 
 
   fprintf(stdout, getResourceString(DBT_OUTPUT_TOPOLOGY_TABLE_HEADER));
@@ -956,15 +965,18 @@ int output_topology(AdmldapInfo ldapInfo,
 	      (const char*)getResourceString(DBT_OUTPUT_TOPOLOGY_DOMAIN_IMAGE),
 	      vals[0]);
       util_ldap_value_free(vals);
+    } else {
+      rc = -1;
+      goto bail;
     }
-    else
-      return -1;
 
     /* HOST */
 
     if((ldapError = sorted_search(HOST_ATTR, server, ldap_get_dn(server, \
                domain_entry), LDAP_SCOPE_ONELEVEL,
-				  HOST_OBJTYPE, NULL, 0, &host_result)) != LDAP_SUCCESS)
-      return -1;
+				  HOST_OBJTYPE, NULL, 0, &host_result)) != LDAP_SUCCESS) {
+      rc = -1;
+      goto bail;
+    }
     
     for(host_entry = ldap_first_entry(server, host_result);
 	host_entry != NULL;
@@ -979,15 +991,18 @@ int output_topology(AdmldapInfo ldapInfo,
 		(const char*)getResourceString(DBT_OUTPUT_TOPOLOGY_HOST_IMAGE),
 		vals[0]);
 	util_ldap_value_free(vals);
+      } else {
+        rc = -1;
+        goto bail;
       }
-      else
-	return -1;
    
       /* SERVER GROUP */
 
       if((ldapError = sorted_search(SERVERGROUP_ATTR, server, ldap_get_dn(server, \
                host_entry), LDAP_SCOPE_ONELEVEL,
-				    SERVERGROUP_OBJTYPE, NULL, 0, &servergroup_result)) != LDAP_SUCCESS)
-	return -1;
+				    SERVERGROUP_OBJTYPE, NULL, 0, &servergroup_result)) != LDAP_SUCCESS) {
+        rc = -1;
+        goto bail;
+      }
 
       first_servergroup = 1;
       for(servergroup_entry = ldap_first_entry(server, servergroup_result);
@@ -1020,15 +1035,18 @@ int output_topology(AdmldapInfo ldapInfo,
 		  (const char*)getResourceString(DBT_OUTPUT_TOPOLOGY_SERVER_GROUP_IMAGE),
 		  vals[0]);
 	  util_ldap_value_free(vals);
-	}
-	else
-	  return -1;
+	} else {
+          rc = -1;
+          goto bail;
+        }
 		
 	/* ISIE */
 	
 	if((ldapError = sorted_search(ISIE_PRODNAME_ATTR, server, ldap_get_dn(server, \
                servergroup_entry), LDAP_SCOPE_ONELEVEL,
-				      ISIE_OBJTYPE, NULL, 0, &isie_result)) != LDAP_SUCCESS)
-	  return -1;
+				      ISIE_OBJTYPE, NULL, 0, &isie_result)) != LDAP_SUCCESS) {
+          rc = -1;
+          goto bail;
+        }
 	
 	for(isie_entry = ldap_first_entry(server, isie_result);
 	    isie_entry != NULL;
@@ -1077,15 +1095,18 @@ int output_topology(AdmldapInfo ldapInfo,
 	    free(version);
 	    util_ldap_value_free(vals);
 	    util_ldap_value_free(vals2);
-	  }
-	  else
-	    return -1;
+	  } else {
+            rc = -1;
+            goto bail;
+          }
 
 	  /* SIE */
 	  
 	  if((ldapError = sorted_search(SIE_SERVERID_ATTR, server, ldap_get_dn(server, \
                isie_entry), LDAP_SCOPE_ONELEVEL,
-					SIE_OBJTYPE, NULL, 0, &sie_result)) != LDAP_SUCCESS)
-	    return -1;
+					SIE_OBJTYPE, NULL, 0, &sie_result)) != LDAP_SUCCESS) {
+            rc = -1;
+            goto bail;
+          }
 	  
 	  for(sie_entry = ldap_first_entry(server, sie_result);
 	      sie_entry != NULL;
@@ -1137,205 +1158,158 @@ int output_topology(AdmldapInfo ldapInfo,
 		  
 	      /* directories to pass info log CGI based on product */
 	      if(strstr(ldap_get_dn(server, sie_entry), "Administration")) {
+	        char *dn_escaped = htmladmin_strdup_escaped(ldap_get_dn(server, \
sie_entry)); +	        char *val_escaped = htmladmin_strdup_escaped(vals[0]);
+
 		running = server_status(server_host, server_port[0]);
 		if(running == 1) {
-		  /* if this ever changes, use PR_smprintf instead of malloc + sprintf */
-		  href = (char *)malloc(strlen(getResourceString(DBT_OUTPUT_TOPOLOGY_SERVER_ENTRY)) \
                +
-					strlen(htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry))) +
-					(view ? strlen(viewparam) : 0) +
-					1);
-		  sprintf(href, 
-			  getResourceString(DBT_OUTPUT_TOPOLOGY_SERVER_ENTRY),
-			  htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry)),
-			  view ? viewparam : "");
+                  href = \
PR_smprintf(getResourceString(DBT_OUTPUT_TOPOLOGY_SERVER_ENTRY), +                    \
dn_escaped, view ? viewparam : "");  }
 
+	        info_link = \
PR_smprintf(getResourceString(DBT_OUTPUT_TOPOLOGY_ADMIN_INFO_LINK), +         	       \
admin_url, dn_escaped); +
+	        log_link = \
PR_smprintf(getResourceString(DBT_OUTPUT_TOPOLOGY_ADMIN_LOG_LINK), +	                 \
admin_url, val_escaped);  
-          /* if this ever changes, use PR_smprintf instead of malloc + sprintf */
-          info_link = (char \
*)malloc(strlen(getResourceString(DBT_OUTPUT_TOPOLOGY_ADMIN_INFO_LINK)) + \
strlen(admin_url) + strlen(htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry))) \
                + 2);
-          sprintf(info_link, 
-                  (const \
                char*)getResourceString(DBT_OUTPUT_TOPOLOGY_ADMIN_INFO_LINK),
-                  admin_url,
-                  htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry)));
-		    
-
-          /* if this ever changes, use PR_smprintf instead of malloc + sprintf */
-          log_link = (char \
*)malloc(strlen(getResourceString(DBT_OUTPUT_TOPOLOGY_ADMIN_LOG_LINK)) + \
                strlen(admin_url) + strlen(htmladmin_strdup_escaped(vals[0])) + 2);
-          sprintf(log_link, 
-                  (const \
                char*)getResourceString(DBT_OUTPUT_TOPOLOGY_ADMIN_LOG_LINK),
-                  admin_url,
-                  htmladmin_strdup_escaped(vals[0]));
-
-		  fprintf(stdout, 
-			  (const char*)getResourceString(DBT_OUTPUT_TOPOLOGY_STATUS),
-			  info_link,
-			  log_link,
-			  (running == 1) ? getResourceString(DBT_OUTPUT_TOPOLOGY_ON) : ((running == -1) ? \
getResourceString(DBT_OUTPUT_TOPOLOGY_UNKNOWN) : \
getResourceString(DBT_OUTPUT_TOPOLOGY_OFF)), +		fprintf(stdout, (const \
char*)getResourceString(DBT_OUTPUT_TOPOLOGY_STATUS), +	                info_link, \
log_link, +	                (running == 1) ? \
getResourceString(DBT_OUTPUT_TOPOLOGY_ON) : ((running == -1) ?  \
getResourceString(DBT_OUTPUT_TOPOLOGY_UNKNOWN) : \
getResourceString(DBT_OUTPUT_TOPOLOGY_OFF)),  (running == 1) ? href : "");
 
-	      }
-	      else if(strstr(ldap_get_dn(server, sie_entry), "Directory")) {
+	        free((void *)dn_escaped);
+	        free((void *)val_escaped);
+	        PR_smprintf_free((char *)href);
+	        PR_smprintf_free((char *)info_link);
+	        PR_smprintf_free((char *)log_link);
+	      } else if(strstr(ldap_get_dn(server, sie_entry), "Directory")) {
+	        /* 
+	         * Directory Server - local config file.
+	         * Can't figure out directories from here so pass in the server id and have \
the CGI guess  +	         */
 	      	char *repl_link;
-		/* 
-		 * Directory Server - local config file.
-		 * Can't figure out directories from here so pass in the server id and have the \
                CGI guess 
-		 */
+	        char *dn_escaped = htmladmin_strdup_escaped(ldap_get_dn(server, \
sie_entry)); +	        char *val_escaped = htmladmin_strdup_escaped(vals[0]);
+	        char *host_escaped = htmladmin_strdup_escaped(host);
 
 		running = server_status(server_host, server_port[0]);
 		if(running == 1) {
 
-          /* if this ever changes, use PR_smprintf instead of malloc + sprintf */
-		  href = (char *)malloc(strlen(getResourceString(DBT_OUTPUT_TOPOLOGY_SERVER_RUNNING)) \
                +
-					strlen(htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry))) +
-					(view ? strlen(viewparam) : 0) +
-					1);
-		  sprintf(href, 
-			  (const char*)getResourceString(DBT_OUTPUT_TOPOLOGY_SERVER_RUNNING),
-			  htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry)),
-			  view ? viewparam : "");
+	          href = PR_smprintf(getResourceString(DBT_OUTPUT_TOPOLOGY_SERVER_RUNNING),
+	                             dn_escaped, view ? viewparam : "");
 		}
 		else if(running == 0) {
-
-          /* if this ever changes, use PR_smprintf instead of malloc + sprintf */
-		  href = (char *)malloc(strlen(getResourceString(DBT_OUTPUT_TOPOLOGY_SERVER_STOP)) \
                +
-					strlen(htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry))) +
-					(view ? strlen(viewparam) : 0) +
-					1);
-		  sprintf(href, (const char*)getResourceString(DBT_OUTPUT_TOPOLOGY_SERVER_STOP),
-			  htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry)),
-			  view ? viewparam : "");
+	          href = PR_smprintf(getResourceString(DBT_OUTPUT_TOPOLOGY_SERVER_STOP),
+	                             dn_escaped, view ? viewparam : "");
 		}
 
-        /* if this ever changes, use PR_smprintf instead of malloc + sprintf */
-        info_link = (char \
*)malloc(strlen(getResourceString(DBT_OUTPUT_TOPOLOGY_DIRECTORY_INFO_LINK)) + \
strlen(admin_url) + strlen(htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry))) \
                + 2);
-        sprintf(info_link, \
                getResourceString(DBT_OUTPUT_TOPOLOGY_DIRECTORY_INFO_LINK),
-                admin_url,
-                htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry)));
-
-        /* if this ever changes, use PR_smprintf instead of malloc + sprintf */
-        log_link = (char \
*)malloc(strlen(getResourceString(DBT_OUTPUT_TOPOLOGY_DIRECTORY_LOG_LINK)) + \
                strlen(admin_url) + strlen(htmladmin_strdup_escaped(vals[0])) + 2);
-        sprintf(log_link, (const \
                char*)getResourceString(DBT_OUTPUT_TOPOLOGY_DIRECTORY_LOG_LINK),
-                admin_url,
-                htmladmin_strdup_escaped(vals[0]));
-
-        /* if this ever changes, use PR_smprintf instead of malloc + sprintf */
-        repl_link = (char \
*)malloc(strlen(getResourceString(DBT_OUTPUT_TOPOLOGY_DIRECTORY_REPL_LINK)) + \
strlen(admin_url)*2 + strlen(htmladmin_strdup_escaped(host)) + /*space for port num*/ \
                12);
-        sprintf(repl_link, (const \
                char*)getResourceString(DBT_OUTPUT_TOPOLOGY_DIRECTORY_REPL_LINK),
-                admin_url,
-                htmladmin_strdup_escaped(host),
-                server_port[0],
-                admin_url);
-
-		fprintf(stdout, 
-			(const char*)getResourceString(DBT_OUTPUT_TOPOLOGY_STATUS_WITH_REPL),
-			repl_link,
-			info_link,
-			log_link,
-			(running == 1) ? getResourceString(DBT_OUTPUT_TOPOLOGY_ON) : ((running == 0) ? \
getResourceString(DBT_OUTPUT_TOPOLOGY_OFF) : \
                getResourceString(DBT_OUTPUT_TOPOLOGY_UNKNOWN)),
-			(running == 1 || running == 0) ?  href : "");		
-	      }
-	      else if(strstr(ldap_get_dn(server, sie_entry), "Enterprise")) {
+	        info_link = \
PR_smprintf(getResourceString(DBT_OUTPUT_TOPOLOGY_DIRECTORY_INFO_LINK), +	            \
admin_url, dn_escaped); +
+	        log_link = \
PR_smprintf(getResourceString(DBT_OUTPUT_TOPOLOGY_DIRECTORY_LOG_LINK), +	             \
admin_url, val_escaped); +
+	        repl_link = \
PR_smprintf(getResourceString(DBT_OUTPUT_TOPOLOGY_DIRECTORY_REPL_LINK), +	            \
admin_url, host_escaped, server_port[0], admin_url); +
+	        fprintf(stdout, 
+	                (const \
char*)getResourceString(DBT_OUTPUT_TOPOLOGY_STATUS_WITH_REPL), +	                \
repl_link, +	                info_link,
+	                log_link,
+	                (running == 1) ? getResourceString(DBT_OUTPUT_TOPOLOGY_ON) : \
((running == 0) ? getResourceString(DBT_OUTPUT_TOPOLOGY_OFF) : \
getResourceString(DBT_OUTPUT_TOPOLOGY_UNKNOWN)), +	                (running == 1 || \
running == 0) ?  href : "");		 +
+	        free((void *)dn_escaped);
+	        free((void *)val_escaped);
+	        free((void *)host_escaped);
+	        PR_smprintf_free((char *)href);
+	        PR_smprintf_free((char *)info_link);
+	        PR_smprintf_free((char *)log_link);
+	        PR_smprintf_free((char *)repl_link);
+	      } else if(strstr(ldap_get_dn(server, sie_entry), "Enterprise")) {
 		/* 
 		 * Enterprise Server - local config file.
 		 * Can't figure out directories from here so pass in the server id and have the \
                CGI guess 
 		 */
+	        char *dn_escaped = htmladmin_strdup_escaped(ldap_get_dn(server, \
sie_entry)); +	        char *val_escaped = htmladmin_strdup_escaped(vals[0]);
 
 		running = server_status(server_host, server_port[0]);
 
 		if(running == 1) {
-
-		  href = (char *)malloc(strlen(getResourceString(DBT_OUTPUT_TOPOLOGY_ES_ON)) +
-					strlen(htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry))) +
-					(view ? strlen(viewparam) : 0) +
-					1);
-		  sprintf(href,
-			  (const char*)getResourceString(DBT_OUTPUT_TOPOLOGY_ES_ON),
-			  htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry)),
-			  view ? viewparam : "");
+	          href = PR_smprintf(getResourceString(DBT_OUTPUT_TOPOLOGY_ES_ON),
+	                             dn_escaped, view ? viewparam : "");
 		}
 		else if(running == 0) {
-
-		  href = (char *)malloc(strlen(getResourceString(DBT_OUTPUT_TOPOLOGY_ES_OFF)) +
-					strlen(htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry))) +
-					(view ? strlen(viewparam) : 0) +
-					1);
-		  sprintf(href, (const char*)getResourceString(DBT_OUTPUT_TOPOLOGY_ES_OFF),
-			  htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry)),
-			  view ? viewparam : "");
+	          href = PR_smprintf(getResourceString(DBT_OUTPUT_TOPOLOGY_ES_OFF),
+	                             dn_escaped, view ? viewparam : "");
 		}		
 
-        info_link = (char \
*)malloc(strlen(getResourceString(DBT_OUTPUT_TOPOLOGY_ES_INFO_LINK)) + \
strlen(admin_url) + strlen(htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry))) \
                + 2);
-        sprintf(info_link, (const \
                char*)getResourceString(DBT_OUTPUT_TOPOLOGY_ES_INFO_LINK),
-                admin_url,
-                htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry)));
-
-        log_link = (char \
*)malloc(strlen(getResourceString(DBT_OUTPUT_TOPOLOGY_ES_LOG_LINK)) + \
                strlen(admin_url) + strlen(htmladmin_strdup_escaped(vals[0])) + 2);
-        sprintf(log_link, (const \
                char*)getResourceString(DBT_OUTPUT_TOPOLOGY_ES_LOG_LINK),
-                admin_url,
-                htmladmin_strdup_escaped(vals[0]));
-
-		  fprintf(stdout, 
-			  (const char*)getResourceString(DBT_OUTPUT_TOPOLOGY_STATUS),
-			  info_link,
-			  log_link,
-			  (running == 1) ? getResourceString(DBT_OUTPUT_TOPOLOGY_ON) : ((running == -1) ? \
getResourceString(DBT_OUTPUT_TOPOLOGY_UNKNOWN) : \
                getResourceString(DBT_OUTPUT_TOPOLOGY_OFF)),
-			  (running == 1) ? href : "");
+	        info_link = \
PR_smprintf(getResourceString(DBT_OUTPUT_TOPOLOGY_ES_INFO_LINK), +	                   \
admin_url, dn_escaped);  
-	      }
-	      else if(strstr(ldap_get_dn(server, sie_entry), "Certificate")) {
+	        log_link = PR_smprintf(getResourceString(DBT_OUTPUT_TOPOLOGY_ES_LOG_LINK),
+	                               admin_url, val_escaped);
+
+	        fprintf(stdout, 
+	                (const char*)getResourceString(DBT_OUTPUT_TOPOLOGY_STATUS),
+	                info_link, log_link,
+	                (running == 1) ? getResourceString(DBT_OUTPUT_TOPOLOGY_ON) : \
((running == -1) ?  getResourceString(DBT_OUTPUT_TOPOLOGY_UNKNOWN) : \
getResourceString(DBT_OUTPUT_TOPOLOGY_OFF)), +	                (running == 1) ? href \
: ""); +
+	        free((void *)dn_escaped);
+	        free((void *)val_escaped);
+	        PR_smprintf_free((char *)href);
+	        PR_smprintf_free((char *)info_link);
+	        PR_smprintf_free((char *)log_link);
+	      } else if(strstr(ldap_get_dn(server, sie_entry), "Certificate")) {
 		/* 
 		 * Certificate Server - local config file.
 		 * Can't figure out directories from here so pass in the server id and have the \
                CGI guess 
 		 */
+		char *dn_escaped = htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry));
+		char *val_escaped = htmladmin_strdup_escaped(vals[0]);
 		
 		running = server_status(server_host, server_port[0]);
 
 		if(running == 1) {
-
-		  href = (char *)malloc(strlen(getResourceString(DBT_OUTPUT_TOPOLOGY_CMS_ON)) +
-					strlen(htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry))) +
-					(view ? strlen(viewparam) : 0) +
-					1);
-		  sprintf(href, (const char*)getResourceString(DBT_OUTPUT_TOPOLOGY_CMS_ON),
-			  htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry)),
-			  view ? viewparam : "");
+		  href = PR_smprintf(getResourceString(DBT_OUTPUT_TOPOLOGY_CMS_ON),
+		                     dn_escaped, view ? viewparam : "");
 		}
 		else if(running == 0) {
-
-		  href = (char *)malloc(strlen(getResourceString(DBT_OUTPUT_TOPOLOGY_CMS_OFF)) +
-					strlen(htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry))) +
-					(view ? strlen(viewparam) : 0) +
-					1);
-		  sprintf(href, (const char*)getResourceString(DBT_OUTPUT_TOPOLOGY_CMS_OFF),
-			  htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry)),
-			  view ? viewparam : "");
+		  href = PR_smprintf(getResourceString(DBT_OUTPUT_TOPOLOGY_CMS_OFF),
+		                     dn_escaped, view ? viewparam : "");
 		}
 
-        info_link = (char \
*)malloc(strlen(getResourceString(DBT_OUTPUT_TOPOLOGY_CMS_INFO_LINK)) + \
strlen(admin_url) + strlen(htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry))) \
                + 2);
-        sprintf(info_link, (const \
                char*)getResourceString(DBT_OUTPUT_TOPOLOGY_CMS_INFO_LINK),
-                admin_url,
-                htmladmin_strdup_escaped(ldap_get_dn(server, sie_entry)));
-
-        log_link = (char \
*)malloc(strlen(getResourceString(DBT_OUTPUT_TOPOLOGY_CMS_LOG_LINK)) + \
                strlen(admin_url) + strlen(htmladmin_strdup_escaped(vals[0])) + 2);
-        sprintf(log_link, (const \
                char*)getResourceString(DBT_OUTPUT_TOPOLOGY_CMS_LOG_LINK),
-                admin_url,
-                htmladmin_strdup_escaped(vals[0]));
-
-		  fprintf(stdout, 
-			  (const char*)getResourceString(DBT_OUTPUT_TOPOLOGY_STATUS),
-			  info_link,
-			  log_link,
-			  (running == 1) ? getResourceString(DBT_OUTPUT_TOPOLOGY_ON) : ((running == -1) ? \
getResourceString(DBT_OUTPUT_TOPOLOGY_UNKNOWN) : \
                getResourceString(DBT_OUTPUT_TOPOLOGY_OFF)),
-			  (running == 1) ? href : "");
+		info_link = PR_smprintf(getResourceString(DBT_OUTPUT_TOPOLOGY_CMS_INFO_LINK),
+		                        admin_url, dn_escaped);
+
+		log_link = PR_smprintf(getResourceString(DBT_OUTPUT_TOPOLOGY_CMS_LOG_LINK),
+		                       admin_url, val_escaped);
 
+		fprintf(stdout, 
+		        (const char*)getResourceString(DBT_OUTPUT_TOPOLOGY_STATUS),
+		        info_link,
+		        log_link,
+		        (running == 1) ? getResourceString(DBT_OUTPUT_TOPOLOGY_ON) : ((running == \
-1) ?  getResourceString(DBT_OUTPUT_TOPOLOGY_UNKNOWN) : \
getResourceString(DBT_OUTPUT_TOPOLOGY_OFF)), +		        (running == 1) ? href : "");
+
+		free((void *)dn_escaped);
+		free((void *)val_escaped);
+		PR_smprintf_free((char *)href);
+		PR_smprintf_free((char *)info_link);
+		PR_smprintf_free((char *)log_link);
 	      }
 
 	      fprintf(stdout, getResourceString(DBT_OUTPUT_TOPOLOGY_TABLE_FOOTER));
 
 	      util_ldap_value_free(vals);
-	    }
-	    else
-	      return -1;
+	    } else {
+              rc = -1;
+              goto bail;
+            }
 
 	  } /* SIE LOOP */
 
@@ -1348,8 +1322,13 @@ int output_topology(AdmldapInfo ldapInfo,
 
   fprintf(stdout, "</table>\n");
 
-  return 0;
+bail:
+  for (i=0; view_list && view_list[i]; i++) {
+    free((void *)view_list[i]);
+  }
+  free((void *)view_list);
 
+  return rc;
 } /* output_topology */
 
 
@@ -1366,7 +1345,7 @@ void start_server(char *admin_url,
   char *admin_port = NULL;
 
   char *buf = strdup(admin_url);
-  char *request;
+  char *request = NULL;
 
   char *cgi_result;
   int errorcode;
@@ -1392,54 +1371,26 @@ void start_server(char *admin_url,
 
   if(service) {
     /* start a messaging request */
-
-    request = (char *)malloc(strlen(getResourceString(DBT_START_SERVER_MESSAGING)) +
-			     strlen(serverid) + 1 +
-			     strlen(service) +
-			     strlen((char *)auth) +
-			     3); /* space + 2 newline characters */
-
-    sprintf(request, (const char*)getResourceString(DBT_START_SERVER_MESSAGING),
-	    serverid,
-	    service,
-	    (char *)auth);
+    request = PR_smprintf(getResourceString(DBT_START_SERVER_MESSAGING),
+                          serverid, service, (char *)auth);
   }
   else {
     if(!strncmp(serverid, "cert-", 5)) {
       int content_length;
+      char *serverid_trimmed_escaped = htmladmin_strdup_escaped(&(serverid[5]));
       
       content_length = strlen("instanceID=") + strlen(serverid) - 5;
          /* minus "cert-" */
 
       /* start a CMS server - why the heck does it need all these parameters ?!?! */
-
-      request = (char *)malloc(strlen(getResourceString(DBT_START_SERVER_CMS)) +
-			       strlen(serverid) +
-			       strlen(admin_host) +
-			       strlen(admin_port) +
-			       strlen((char *)auth) +
-			       5 + /* can't be >5 digits for content length */
-			       strlen(htmladmin_strdup_escaped(serverid)) +
-			       3); /* space + 2 newline characters */
-      
- sprintf(request, (const char*)getResourceString(DBT_START_SERVER_CMS),
-	      serverid,
-	      admin_host,
-	      atoi(admin_port),
-	      (char *)auth,
-	      content_length,
-	      htmladmin_strdup_escaped(&(serverid[5]))); /* takes everything after "cert-" \
                */
-    }
-    else {
-
-      request = (char *)malloc(strlen(getResourceString(DBT_START_SERVER_AS)) +
-			       strlen(serverid) + 1 +
-			       strlen((char *)auth) +
-			       3); /* space + 2 newline characters */
-      
-      sprintf(request, getResourceString(DBT_START_SERVER_AS),
-	      serverid,
-	      (char *)auth);
+      request = PR_smprintf(getResourceString(DBT_START_SERVER_CMS), serverid,
+                            admin_host, atoi(admin_port), (char *)auth,
+                            content_length, serverid_trimmed_escaped);
+
+      free((void *)serverid_trimmed_escaped);
+    } else {
+      request = PR_smprintf(getResourceString(DBT_START_SERVER_AS),
+                            serverid, (char *)auth);
     }
   }
 
@@ -1451,15 +1402,19 @@ void start_server(char *admin_url,
 
   
   if (sockd == NULL) {
-    return;
+    goto bail;
   }
 
   if (parse_http_header(sockd, nbuf, "Administrator") < 0) {
-    return;
+    goto bail;
   }
 
   while( (cgi_result = get_line_from_fd(sockd, nbuf)) != (char *) NULL)  {
   }
+
+bail:
+  free((void *)auth);
+  PR_smprintf_free((char *)request);
   return;
 }
 
@@ -1475,7 +1430,7 @@ void stop_server(char *admin_url,
   char *admin_port = NULL;
 
   char *buf = strdup(admin_url);
-  char *request;
+  char *request = NULL;
 
   char *cgi_result;
   int errorcode;
@@ -1503,55 +1458,26 @@ void stop_server(char *admin_url,
   if(service) {
     /* stop a messaging service */
 
-    request = (char *)malloc(strlen(getResourceString(DBT_STOP_SERVER_MESSAGING)) +
-			     strlen(serverid) + 1 +
-			     strlen(service) +
-			     strlen((char *)auth) +
-			     3); /* space + 2 newline characters */
-
-    sprintf(request, (const char*)getResourceString(DBT_STOP_SERVER_MESSAGING),
-	    serverid,
-	    service,
-	    (char *)auth);
+    request = PR_smprintf(getResourceString(DBT_STOP_SERVER_MESSAGING),
+                          serverid, service, (char *)auth);
   }
   else {
     if(!strncmp(serverid, "cert-", 5)) {
       int content_length;
+      char *serverid_trimmed_escaped = htmladmin_strdup_escaped(&(serverid[5]));
 
       content_length = strlen("instanceID=") + strlen(serverid) - 5;
         /* minus "cert-" */
 
       /* stop a CMS server - why the heck does it need all these parameters ?!?! */
-
-      request = (char *)malloc(strlen(getResourceString(DBT_STOP_SERVER_CMS)) +
-			       strlen(serverid) +
-			       strlen(admin_host) +
-			       strlen(admin_port) +
-			       strlen((char *)auth) +
-			       5 + /* can't be >10 digits for content length */
-			       strlen(htmladmin_strdup_escaped(serverid)) +
-			       3); /* space + 2 newline characters */
-
-
-      sprintf(request, (const char*)getResourceString(DBT_STOP_SERVER_CMS),
-	      serverid,
-	      admin_host,
-	      atoi(admin_port),
-	      (char *)auth,
-	      content_length,
-	      htmladmin_strdup_escaped(&(serverid[5]))); /* takes everything after "cert-" \
                */
-    }
-    else {
-
-      request = (char *)malloc(strlen(getResourceString(DBT_STOP_SERVER_AS)) +
-			       strlen(serverid) + 1 +
-			       strlen((char *)auth) +
-			       3); /* space + 2 newline characters */
-      
-
-      sprintf(request, (const char*)getResourceString(DBT_STOP_SERVER_AS),
-	      serverid,
-	      (char *)auth);
+      request = PR_smprintf(getResourceString(DBT_STOP_SERVER_CMS),
+                            serverid, admin_host, atoi(admin_port),
+                            (char *)auth, content_length, serverid_trimmed_escaped);
+
+      free((void *)serverid_trimmed_escaped);
+    } else {
+      request = PR_smprintf(getResourceString(DBT_STOP_SERVER_AS),
+                            serverid, (char *)auth);
     }
   }
 
@@ -1562,15 +1488,19 @@ void stop_server(char *admin_url,
   }
   
   if (sockd == NULL) {
-    return;
+    goto bail;
   }
 
   if (parse_http_header(sockd, nbuf, "Administrator") < 0) {
-    return;
+    goto bail;
   }
 
   while( (cgi_result = get_line_from_fd(sockd, nbuf)) != (char *) NULL)  {
   }
+
+bail:
+  PR_smprintf_free((char *)request);
+  free((void *)auth);
   return;
 }
 


commit 3a150393a111e9824a20d56b9eb91e44713d90e8
Author: Nathan Kinder <nkinder@redhat.com>
Date:   Thu May 5 11:42:51 2011 -0700

    Bug 702150 - (cov#10817) Leak of string in libdsa
    
    When line-wrapping is used in ds_display_tail(), we leak the wrapped
    content.  We need to free this before it goes out of scope.

diff --git a/lib/libdsa/dsalib_tailf.c b/lib/libdsa/dsalib_tailf.c
index 35b227b..a3fe941 100644
--- a/lib/libdsa/dsalib_tailf.c
+++ b/lib/libdsa/dsalib_tailf.c
@@ -195,6 +195,7 @@ ds_display_tail(char *fileName, int timeOut, int startSeek, char \
*doneMsg,  wrapBuf = wrapLines( msgBuf );
 		    if ( wrapBuf != NULL ) {
 			ds_send_error(wrapBuf, 5);
+		        free((void *)wrapBuf);
 		    } else {
 			ds_send_error(msgBuf, 5);
 		    }


commit f4596e50cdd883e3c30342246e248b65de343dc8
Author: Nathan Kinder <nkinder@redhat.com>
Date:   Thu May 5 11:38:34 2011 -0700

    Bug 702150 - (cov#10816) file descriptor leak in dsalib
    
    We leak a file descriptor in dsalib.  We need to close dirp
    before bailing out of the function.

diff --git a/lib/libdsa/dsalib_util.c b/lib/libdsa/dsalib_util.c
index 15eb512..8f2fc8f 100644
--- a/lib/libdsa/dsalib_util.c
+++ b/lib/libdsa/dsalib_util.c
@@ -105,6 +105,7 @@ ds_get_file_list( char *dir )
     }
     
     if (( ret = malloc( sizeof( char * ))) == NULL ) {
+	closedir(dirp);
 	return NULL;
     };
 


commit 5c72311cc978d2deacf5cb840f34d86eb4250c11
Author: Nathan Kinder <nkinder@redhat.com>
Date:   Thu May 5 11:32:47 2011 -0700

    Bug 702150 - leak of config array in dsalib
    
    The ds_get_config() function in dsalib allocates a config list,
    but none of the callers free the returned list.  This adds a free
    function and makes all callers of ds_get_config() free the list.
    
    The function that builds the config list also needs to free the
    list if it encounters an error before bailing out.
    
    cov#10818,10815,10814

diff --git a/admserv/cgi-ds/ds_listdb.c b/admserv/cgi-ds/ds_listdb.c
index 14afd82..3fe9024 100644
--- a/admserv/cgi-ds/ds_listdb.c
+++ b/admserv/cgi-ds/ds_listdb.c
@@ -34,9 +34,12 @@
 int main(int argc, char *argv[], char *envp[])
 {
     char **bak_dirs;
+    char **ds_config = NULL;
     int i = 0;
 
-    ds_become_localuser (ds_get_config (DS_REAL_CONFIG));
+    ds_config = ds_get_config (DS_REAL_CONFIG);
+    ds_become_localuser(ds_config);
+    ds_free_config(ds_config);
 
     /* Tell the receiver we are about to start sending data */
     fprintf(stdout, "\n");
diff --git a/include/libdsa/dsalib.h b/include/libdsa/dsalib.h
index de97b95..eddf28a 100644
--- a/include/libdsa/dsalib.h
+++ b/include/libdsa/dsalib.h
@@ -285,6 +285,7 @@ extern DS_EXPORT_SYMBOL void ds_display_tail(char *fileName, int \
timeOut,  int startSeek, char *doneMsg, char *lastLine);
 extern DS_EXPORT_SYMBOL char **ds_get_bak_dirs();
 extern DS_EXPORT_SYMBOL char **ds_get_config(int type);
+extern DS_EXPORT_SYMBOL void ds_free_config(char **conf_list);
 extern DS_EXPORT_SYMBOL char *ds_get_config_dir();
 extern DS_EXPORT_SYMBOL void ds_set_config_dir(char *config_dir);
 extern DS_EXPORT_SYMBOL char *ds_get_run_dir();
diff --git a/lib/libdsa/dsalib_conf.c b/lib/libdsa/dsalib_conf.c
index 41c8204..4673e2f 100644
--- a/lib/libdsa/dsalib_conf.c
+++ b/lib/libdsa/dsalib_conf.c
@@ -96,6 +96,21 @@ ds_get_config(int type)
 }
 
 /*
+ * Frees the config list returned by ds_get_config().
+ */
+DS_EXPORT_SYMBOL void
+ds_free_config(char **conf_list)
+{
+    int i;
+
+    for (i=0; conf_list && conf_list[i]; i++) {
+        PR_smprintf_free(conf_list[i]);
+    }
+
+    free((void *)conf_list);
+}
+
+/*
  * NOTE: the ordering of the following array elements must be kept in sync
  * with the ordering of the #defines in ../include/dsalib.h.
  */
@@ -147,8 +162,10 @@ ds_get_config_value( int option )
 		++value;
 	}
 	if ( !strcasecmp( attr, all[ i ] )) {
+            ds_free_config(all);
 	    return strdup( value );
 	}
     }
+    ds_free_config(all);
     return NULL;
 }
diff --git a/lib/libdsa/dsalib_confs.c b/lib/libdsa/dsalib_confs.c
index 0747fe9..3c24c98 100644
--- a/lib/libdsa/dsalib_confs.c
+++ b/lib/libdsa/dsalib_confs.c
@@ -76,6 +76,7 @@ ds_get_conf_from_file(FILE *conf)
     char        **conf_list = NULL;
     char *entry = 0;
     int lineno = 0;
+    int i = 0;
 #if defined(USE_OPENLDAP)
     int buflen = 0;
 #endif
@@ -103,6 +104,10 @@ ds_get_conf_from_file(FILE *conf)
 		{
 		    ds_send_error("Unknown error processing config file", 0);
 		    free(begin);
+		    for (i=0; conf_list && conf_list[i]; i++) {
+		        PR_smprintf_free(conf_list[i]);
+		    }
+		    free((void *)conf_list);
 		    return NULL;
 		}
 		listsize++;
diff --git a/lib/libdsa/dsalib_location.c b/lib/libdsa/dsalib_location.c
index 7b4bfff..1ed29ea 100644
--- a/lib/libdsa/dsalib_location.c
+++ b/lib/libdsa/dsalib_location.c
@@ -305,15 +305,18 @@ ds_get_logfile_name(int config_type)
         /* For DS 4.0, no error output if file doesn't exist - that's
            a normal situation */
         /* ds_send_error("ds_get_logfile_name: filename == NULL", 0); */
+        ds_free_config(ds_config);
         return(NULL);
     }
     if ( ((int) strlen(filename)) >= PATH_MAX ) {
         ds_send_error("ds_get_logfile_name: filename too long", 0);
         free(filename);
+        ds_free_config(ds_config);
         return(NULL);
     }
     PL_strncpyz(logfile, filename, sizeof(logfile));
     free(filename);
+    ds_free_config(ds_config);
     return(logfile);
 }
 


commit ed6de71b4996f0630a617233562d4a2b88314236
Author: Nathan Kinder <nkinder@redhat.com>
Date:   Thu May 5 11:00:58 2011 -0700

    Bug 702150 - (cov#10820,10819) file descriptor leaks in readlog cgi
    
    The readlog cgi leaks file descriptors in two places.  We need to
    close the file descriptors when exiting the functions.

diff --git a/admserv/cgi-src40/ReadLog.c b/admserv/cgi-src40/ReadLog.c
index 8212660..17e1018 100644
--- a/admserv/cgi-src40/ReadLog.c
+++ b/admserv/cgi-src40/ReadLog.c
@@ -58,13 +58,14 @@ int readLogFile(char *filename, int startEntry, int endEntry)
 	{
 		while(fgets(line, BIG_LINE, f))
 		{
-            if(strstr(line, "format=") == NULL)
-            {
-                if((entryIndex >= startEntry) && (entryIndex <= endEntry))
-                    fprintf(stdout, "%s", line);
-                entryIndex++;
-            }
+			if(strstr(line, "format=") == NULL)
+			{
+				if((entryIndex >= startEntry) && (entryIndex <= endEntry))
+					fprintf(stdout, "%s", line);
+				entryIndex++;
+			}
 		}
+		fclose(f);
 		return 0;
 	}
 	return 1;
@@ -132,10 +133,11 @@ int getEntryCount(char *filename, int *value)
 	{
 		while(fgets(line, BIG_LINE, f) != NULL)
 		{
-            if(strstr(line, "format=") == NULL)
-                entryIndex++;
+			if(strstr(line, "format=") == NULL)
+				entryIndex++;
 		}
 		*value = entryIndex;
+		fclose(f);
 		return 0;
 	}	
 	return 1;


commit 7df0ab829f01f02bcff670c364ed14c6cf12b935
Author: Nathan Kinder <nkinder@redhat.com>
Date:   Thu May 5 10:55:02 2011 -0700

    Bug 702150 - (cov#10822,10821) file descriptor leaks in config cgi
    
    The config cgi leaks fiel descriptors in two places.  We need to
    close the file descriptors when exiting the functions.

diff --git a/admserv/cgi-src40/config.c b/admserv/cgi-src40/config.c
index b068f60..fb0188e 100644
--- a/admserv/cgi-src40/config.c
+++ b/admserv/cgi-src40/config.c
@@ -748,6 +748,7 @@ static int update_conf(char *file, char *name, char *val) {
   int linecnt=0;	
   char *lines[CONF_LINES];
   char *configdir = util_get_conf_dir();
+  int rc = 0;
 
   util_find_file_in_paths(filename, sizeof(filename), file, configdir, "", \
"admin-serv/config");  
@@ -759,8 +760,11 @@ static int update_conf(char *file, char *name, char *val) {
   }
 
   while(fgets(inbuf, sizeof(inbuf), f) != NULL) {
-    if (linecnt >= CONF_LINES)
-       return 1; /* our buffer ran out, give up */
+    if (linecnt >= CONF_LINES) {
+       rc = 1; /* our buffer ran out, give up */
+       goto bail;
+    }
+
     if (strncasecmp(inbuf,name,strlen(name)) == 0) { /* Line starts with the \
attribute name */  if(val && *val != '\0') {
 	PR_snprintf(buf, sizeof(buf), "%s %s\n", name, val);
@@ -793,9 +797,9 @@ static int update_conf(char *file, char *name, char *val) {
     fprintf(f, "%s", lines[i]);
   }
 
+bail:
   fclose(f);
-
-  return 0;
+  return rc;
 }
 
 
@@ -985,6 +989,7 @@ static int change_uid_all(char *dir, int olduid, int newuid) {
     PR_snprintf(fpath, sizeof(fpath), "%s/%s", dir, dp->d_name);
 
     if (stat(fpath, &filestat) != 0) {
+      closedir(dirp);
       PR_snprintf(buf, sizeof(buf), "Change Server UID: stat(%s) failed, \
errno=%d\n", fpath, errno);  rpt_err(SYSTEM_ERROR, buf, NULL, NULL);
       return -1;


commit bd6f9522d70f4c7117422fe23ee1171ad550a5d8
Author: Nathan Kinder <nkinder@redhat.com>
Date:   Thu May 5 10:43:04 2011 -0700

    Bug 702150 - (cov#10823) File descriptors leaked in help cgi
    
    The help cgi leaks file descriptors.  We need to be sure to close
    each file we open.

diff --git a/admserv/cgi-src40/help.c b/admserv/cgi-src40/help.c
index f673492..1d59ba1 100644
--- a/admserv/cgi-src40/help.c
+++ b/admserv/cgi-src40/help.c
@@ -314,6 +314,8 @@ no_frame_help(char *name[], char *val[], int cnt, char *product, \
char *content)  fputs(path, stdout);
    }
 
+   fclose(file);
+
    /* Open the target file and return the contents */
    safe_snprintf(path, sizeof(path), "%s%c%s%c%s%c%s", MANUALDIR, FILE_SEP, locale, \
FILE_SEP,  product, FILE_SEP, content);
@@ -336,6 +338,8 @@ no_frame_help(char *name[], char *val[], int cnt, char *product, \
char *content)  fputs(path, stdout);
    }
 
+   fclose(file);
+
    /* read and flush the footer to stdout */
    safe_snprintf(path, sizeof(path), "%s%c%s%c%s%c%s", MANUALDIR, FILE_SEP, locale, \
FILE_SEP,  product, FILE_SEP, FOOTER_FILE);


--
389 commits mailing list
389-commits@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/389-commits


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic