[Fedora-directory-commits] adminutil/lib/libadminutil admutil.c, 1.9, 1.10 psetc.c, 1.5, 1.6 srvutil.c, 1.6, 1.7 uginfo.c, 1.5, 1.6

[Noriko Hosoi]

Author: nhosoi

Update of /cvs/dirsec/adminutil/lib/libadminutil
In directory cvs1.fedora.phx.redhat.com:/tmp/cvs-serv32529/lib/libadminutil

Modified Files:
	admutil.c psetc.c srvutil.c uginfo.c 
Log Message:
Resolves: #191834
Summary: Clean up admin password in memory when it's freed
Description: (comment #5)
1) overwrote password string with '\0's.
2) psetCreate (psetc.c), psetCreateSSL (psetcssl.c)
Both has the similar code "passwd = bindPasswd; /* not to free bindPasswd */". 
According to the comment, by setting bindPasswd to passwd, bindPasswd is not
supposed to be freed.  But the current location does not stop it's being freed
since at that point bindPasswd is NULL and NULL is set to passwd.  (Probably,
the path is not usually taken.)



Index: admutil.c
===================================================================
RCS file: /cvs/dirsec/adminutil/lib/libadminutil/admutil.c,v
retrieving revision 1.9
retrieving revision 1.10
diff -u -r1.9 -r1.10
--- admutil.c	5 Jul 2007 21:12:55 -0000	1.9
+++ admutil.c	28 Jan 2009 00:01:10 -0000	1.10
@@ -1434,24 +1434,23 @@
       admInfo->localAdminName = NULL;
     }
     if (admInfo->localAdminPassword) {
+      memset(admInfo->localAdminPassword, '\0', strlen(admInfo->localAdminPassword));
       PR_Free(admInfo->localAdminPassword);
       admInfo->localAdminPassword = NULL;
     }
-    if (admInfo->sieDN)
-      {
-        PR_Free(admInfo->sieDN);
-        admInfo->sieDN = NULL;
-      }
-    if (admInfo->userDN)
-      {
-        PR_Free(admInfo->userDN);
-        admInfo->userDN = NULL;
-      }
-    if (admInfo->passwd)
-      {
-        PR_Free(admInfo->passwd);
-        admInfo->passwd = NULL;
-      }
+    if (admInfo->sieDN) {
+      PR_Free(admInfo->sieDN);
+      admInfo->sieDN = NULL;
+    }
+    if (admInfo->userDN) {
+      PR_Free(admInfo->userDN);
+      admInfo->userDN = NULL;
+    }
+    if (admInfo->passwd) {
+      memset(admInfo->passwd, '\0', strlen(admInfo->passwd));
+      PR_Free(admInfo->passwd);
+      admInfo->passwd = NULL;
+    }
     if (admInfo->ldapHndl) {
       ldap_unbind(admInfo->ldapHndl);
       admInfo->ldapHndl = NULL;
@@ -1876,7 +1875,10 @@
 PR_IMPLEMENT(void)
 admSetCachedSIEPWD(const char *pwd)
 {
-  if (cachedSIEPWD) PR_Free(cachedSIEPWD);
+  if (cachedSIEPWD) {
+    memset(cachedSIEPWD, '\0', strlen(cachedSIEPWD));
+    PR_Free(cachedSIEPWD);
+  }
 
   cachedSIEPWD = PL_strdup(pwd);
 }


Index: psetc.c
===================================================================
RCS file: /cvs/dirsec/adminutil/lib/libadminutil/psetc.c,v
retrieving revision 1.5
retrieving revision 1.6
diff -u -r1.5 -r1.6
--- psetc.c	8 May 2007 19:13:25 -0000	1.5
+++ psetc.c	28 Jan 2009 00:01:10 -0000	1.6
@@ -171,7 +171,10 @@
     if (psetp->configFile) PR_Free(psetp->configFile);
     if (psetp->sieDN) PR_Free(psetp->sieDN);
     if (psetp->binddn) PR_Free(psetp->binddn);
-    if (psetp->bindpw) PR_Free(psetp->bindpw);
+    if (psetp->bindpw) {
+      memset(psetp->bindpw, 0, strlen(psetp->bindpw));
+      PR_Free(psetp->bindpw);
+    }
   
     PR_Free(psetp);
   }
@@ -1362,11 +1365,11 @@
   userDN = admldapGetUserDN(ldapInfo, user);
   if (passwd) {
     bindPasswd = passwd;
-  } else {
-    bindPasswd = admldapGetSIEPWD(ldapInfo);
+  } else { /* passwd is NULL */
+    bindPasswd = admldapGetSIEPWD(ldapInfo); /* duplicated; need to free */
     if (!bindPasswd) {
+      ADM_GetCurrentPassword(errorcode, &bindPasswd); /* should not free */
       passwd = bindPasswd; /* setting this not to free bindPasswd */
-      ADM_GetCurrentPassword(errorcode, &bindPasswd);
     }
   }
 
@@ -1384,7 +1387,13 @@
   PR_Free(sieDN);
   PR_smprintf_free(path);
   PR_Free(userDN);
-  if (!passwd) { if (bindPasswd) PR_Free(bindPasswd); }
+  if (!passwd) {
+    if (bindPasswd) {
+      memset(bindPasswd, '\0', strlen(bindPasswd));
+      PR_Free(bindPasswd);
+      bindPasswd = NULL;
+    }
+  }
   destroyAdmldap(ldapInfo);
   return pset;
 }
@@ -2367,7 +2376,10 @@
   if (pset->binddn) PR_Free(pset->binddn);
   if (userDN) pset->binddn = PL_strdup(userDN);
   else  pset->binddn = NULL;
-  if (pset->bindpw) PR_Free(pset->bindpw);
+  if (pset->bindpw) {
+    memset(pset->bindpw, 0, strlen(pset->bindpw));
+    PR_Free(pset->bindpw);
+  }
   if (passwd) pset->bindpw = PL_strdup(passwd);
   else pset->bindpw = NULL;
 


Index: srvutil.c
===================================================================
RCS file: /cvs/dirsec/adminutil/lib/libadminutil/srvutil.c,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -r1.6 -r1.7
--- srvutil.c	3 Dec 2008 18:36:49 -0000	1.6
+++ srvutil.c	28 Jan 2009 00:01:10 -0000	1.7
@@ -82,7 +82,10 @@
   if (sie) PR_Free(sie);
   if (domainDN) PR_Free(domainDN);
   if (host) PR_Free(host);
-  if (siepwd) PR_Free(siepwd);
+  if (siepwd) {
+    memset(siepwd, '\0', strlen(siepwd));
+    PR_Free(siepwd);
+  }
   return nl;
 
 err:
@@ -90,7 +93,10 @@
   if (sie) PR_Free(sie);
   if (domainDN) PR_Free(domainDN);
   if (host) PR_Free(host);
-  if (siepwd) PR_Free(siepwd);
+  if (siepwd) {
+    memset(siepwd, '\0', strlen(siepwd));
+    PR_Free(siepwd);
+  }
   return NULL;
 }
 
@@ -182,7 +188,10 @@
   psetDelete(domainPset);
   PL_strfree(host);
   PL_strfree(sie);
-  PL_strfree(siepwd);
+  if (siepwd) {
+    memset(siepwd, '\0', strlen(siepwd));
+    PL_strfree(siepwd);
+  }
   PL_strfree(isie);
 
   return resultList;


Index: uginfo.c
===================================================================
RCS file: /cvs/dirsec/adminutil/lib/libadminutil/uginfo.c,v
retrieving revision 1.5
retrieving revision 1.6
diff -u -r1.5 -r1.6
--- uginfo.c	8 May 2007 19:13:25 -0000	1.5
+++ uginfo.c	28 Jan 2009 00:01:10 -0000	1.6
@@ -123,7 +123,7 @@
     if (s && strcmp(s[0], "")) {
       char *temp = strrchr(directoryURLVals[0], '/');
       /* append failover list to url */
-	  if (NULL != temp) {
+      if (NULL != temp) {
         *temp = '\0';
         PR_snprintf(buffer, sizeof(buffer), "%s %s/%s",
                     directoryURLVals[0], s[0], temp + 1);
@@ -144,6 +144,9 @@
   }
   if (bindPasswordVals) {
     *bindPassword = PL_strdup(bindPasswordVals[0]);
+    if (bindPasswordVals[0]) {
+      memset(bindPasswordVals[0], '\0', strlen(bindPasswordVals[0]));
+    }
     ldap_value_free(bindPasswordVals);
   }
   if (directoryInfoRefVals) {
@@ -282,7 +285,11 @@
     }
     if (oldDirectoryURL) PR_Free(oldDirectoryURL);
     if (oldBindDN) PR_Free(oldBindDN);
-    if (oldBindPassword) PR_Free(oldBindPassword);
+    if (oldBindPassword) {
+      memset(oldBindPassword, '\0', strlen(oldBindPassword));
+      PR_Free(oldBindPassword);
+      oldBindPassword = NULL;
+    }
     if (oldDirectoryInfoRef) PR_Free(oldDirectoryInfoRef);
   }
 
@@ -302,7 +309,11 @@
           *error_code = UG_LDAP_SYSTEM_ERR;
           if (oldDirectoryURL) PR_Free(oldDirectoryURL);
           if (oldBindDN) PR_Free(oldBindDN);
-          if (oldBindPassword) PR_Free(oldBindPassword);
+          if (oldBindPassword) {
+            memset(oldBindPassword, '\0', strlen(oldBindPassword));
+            PR_Free(oldBindPassword);
+            oldBindPassword = NULL;
+          }
           if (oldDirectoryInfoRef) PR_Free(oldDirectoryInfoRef);
           return 0;
   }
@@ -449,6 +460,7 @@
     oldBindDN = NULL;
   }
   if (oldBindPassword) {
+    memset(oldBindPassword, '\0', strlen(oldBindPassword));
     PR_Free(oldBindPassword);
     oldBindPassword = NULL;
   }