[Fedora-directory-commits] adminserver/admserv/user-forms/src enduser.c, 1.4, 1.5 index.c, 1.3, 1.4

Richard Allen Megginson (rmeggins) fedora-directory-commits at redhat.com
Fri Mar 31 22:58:23 UTC 2006


Author: rmeggins

Update of /cvs/dirsec/adminserver/admserv/user-forms/src
In directory cvs-int.fedora.redhat.com:/tmp/cvs-serv28761/adminserver/admserv/user-forms/src

Modified Files:
	enduser.c index.c 
Log Message:
Bug(s) fixed: 186280
Bug Description: adminserver: Close potential security vulnerabilities 
in CGI code
Reviewed by: Rob, Pete, Nathan, Noriko (Thanks!)
Fix Description: Most of this just involves making sure that we use 
PR_snprintf/PL_strncpyz/PL_strcatn where able, or just making sure we 
use snprintf/strncpy/strncat correctly and null terminate the buffers.  
I also got rid of some dead code, unused variables, and the like.  There 
are a few cases that are more complex that I have specified below.  In 
some cases I had to change the function signature to add a size 
parameter in cases where the function was copying to a given char * and 
the size was assumed (in most cases this was safe but it's still dangerous).
Platforms tested: Fedora Core 5
Flag Day: no
Doc impact: no



Index: enduser.c
===================================================================
RCS file: /cvs/dirsec/adminserver/admserv/user-forms/src/enduser.c,v
retrieving revision 1.4
retrieving revision 1.5
diff -u -r1.4 -r1.5
--- enduser.c	18 Aug 2005 19:10:57 -0000	1.4
+++ enduser.c	31 Mar 2006 22:58:21 -0000	1.5
@@ -138,11 +138,12 @@
 	BerElement *ber;
 
    if (getenv("HTTP_ACCEPT_LANGUAGE") != NULL)	{
-   	strcpy(langList, getenv("HTTP_ACCEPT_LANGUAGE"));
+       strncpy(langList, getenv("HTTP_ACCEPT_LANGUAGE"), sizeof(langList));
    }
    else {
    	strcpy(langList, "en");
    }
+   langList[sizeof(langList)-1] = 0;
 
 /*  Replace spaces with commas */
    for(ptr=langList; *ptr=='\0'; ptr++)
@@ -153,19 +154,22 @@
 				a = ldap_next_attribute(ld, e, ber)) {
 		if (((ptr = strstr(a, ";lang-en")) != NULL) && (!enLangAttr)) {
 			enLangAttr = 1;
-			strcpy(defaultEnLang, a);
+			strncpy(defaultEnLang, a, sizeof(defaultEnLang));
+            defaultEnLang[sizeof(defaultEnLang)-1] = 0;
 		}
 		
 		if ((ptr = strstr(a, ";lang-")) == NULL) {
 			noLangAttr = 1;
-			strcpy(defaultNoLang, a);
+			strncpy(defaultNoLang, a, sizeof(defaultNoLang));
+            defaultNoLang[sizeof(defaultNoLang)-1] = 0;
 			continue;
 		}
 		else {
 			ptr = ptr + strlen(";lang-");
 			if ((ptr2 = strchr(ptr, ';')) != NULL)
 				*ptr2 = '\0';
-			sprintf(langTag, "%s", ptr);
+			snprintf(langTag, sizeof(langTag), "%s", ptr);
+            langTag[sizeof(langTag)-1] = 0;
 			if (strstr(langList, langTag)) {
 				ldap_memfree(a);
 				ldap_ber_free(ber, 0);
@@ -397,12 +401,13 @@
 		return ldap_init(ldapHost, ldapPort);
 }
 
-/* returns the searchDN underwhich to look for the endUserHtmlIndex programs */
-char *getSearchDN(AdmldapInfo ldapInfo) {
+/* returns the searchDN underwhich to look for the endUserHtmlIndex programs in entrydn */
+void getSearchDN(AdmldapInfo ldapInfo, char *entrydn, size_t size) {
 	char buf[1000];
 	int i, count = 0;
 
-	strcpy(buf, admldapGetSIEDN(ldapInfo));
+	strncpy(buf, admldapGetSIEDN(ldapInfo), sizeof(buf));
+    buf[sizeof(buf)-1] = 0;
 	if (!buf) {
             report_err( DBT_errorBuildingLdapInfo_);
             exit(0);
@@ -421,8 +426,9 @@
    }
    /*Eliminate leading blanks */
 	while (buf[++i] == ' ');
-	sprintf(buf, "%s%c%s", DS_GLOBAL_PREF_RDN, ',', &buf[i]);
-	return buf;
+	snprintf(entrydn, size, "%s%c%s", DS_GLOBAL_PREF_RDN, ',', &buf[i]);
+    entrydn[size-1] = 0;
+	return;
 }
 	
 catlist read_catlist_DS(void)
@@ -460,8 +466,9 @@
 		/* binddn=(char*)malloc(sizeof(char)*100); */
 		/* strcpy(binddn,"uid=jwalker,ou=people,o=airius.com"); */
 
-		sprintf(admconf, "%s%c%s%c%s", nsroot, FILE_PATHSEP, "admin-serv",
+		snprintf(admconf, sizeof(admconf), "%s%c%s%c%s", nsroot, FILE_PATHSEP, "admin-serv",
 			FILE_PATHSEP, "config");
+        admconf[sizeof(admconf)-1] = 0;
 
 		ldapInfo = admldapBuildInfo(admconf, &rv);
   
@@ -472,8 +479,10 @@
 		else {
 			ldapHost = admldapGetHost(ldapInfo);
 			ldapPort = admldapGetPort(ldapInfo);
-                        strcpy(binddn, admldapGetSIEDN(ldapInfo));
-                        strcpy(bindpw, admldapGetSIEPWD(ldapInfo));
+                        strncpy(binddn, admldapGetSIEDN(ldapInfo), sizeof(binddn));
+                        binddn[sizeof(binddn)-1] = 0;
+                        strncpy(bindpw, admldapGetSIEPWD(ldapInfo), sizeof(bindpw));
+                        bindpw[sizeof(bindpw)-1] = 0;
                         bindInfo.binddn = binddn;
                         bindInfo.bindpwd = bindpw;
 
@@ -485,7 +494,7 @@
 			
 					attr[0]="nsAdminEndUserHTMLIndex";
 					attr[1]=NULL;
-					strcpy(EntryDN, getSearchDN(ldapInfo));
+					getSearchDN(ldapInfo, EntryDN, sizeof(EntryDN));
 	
 					if ( i = ldap_search_s(ld, EntryDN, LDAP_SCOPE_SUBTREE, "(nsadminenduserhtmlindex=*)", attr, 0, &result)==LDAP_SUCCESS)
 					{
@@ -532,7 +541,7 @@
     char *tmp=NULL, *data=NULL, *div=NULL;
     char *root = getenv("NETSITE_ROOT");
     
-    GetFileForLanguage(CATLIST_FILE,GetClientLanguage(),existingpath);
+    GetFileForLanguage(CATLIST_FILE,GetClientLanguage(),existingpath,sizeof(existingpath));
     f = fopen(existingpath, "r");
     if(!f)  {
         report_err( DBT_errorCouldNotOpenServersListFile_);
@@ -602,8 +611,9 @@
             if(!data) continue; /* huh?! */
             data++;
            
-            sprintf(path, INCLUDE_PATH, root, data);
-            GetFileForLanguage(path,GetClientLanguage(),existingpath);
+            snprintf(path, sizeof(path), INCLUDE_PATH, root, data);
+            path[sizeof(path)-1] = 0;
+            GetFileForLanguage(path,GetClientLanguage(),existingpath,sizeof(existingpath));
             if (!(flst = fopen(existingpath, "r"))) {
               fprintf(stdout,
                       /*XP_GetClientStr(*/"%s",DBT_errorCouldNotOpenSServerListFile_/*)*/,
@@ -612,7 +622,7 @@
             }
             else 
             {
-              while(fgets(opt, 128, flst) != (char *) NULL)
+              while(fgets(opt, sizeof(opt), flst) != (char *) NULL)
               { 
                 if(!strncmp(opt, CATEGORY, strlen(CATEGORY)))  {
                   tmp=strchr(opt, '\n');
@@ -748,11 +758,12 @@
 
     if (user)
     {
-            sprintf(title,DBT_serverAccountManagementFor_,user);
+            snprintf(title,sizeof(title), DBT_serverAccountManagementFor_,user);
     } else
     {
-            sprintf(title,DBT_serverAccountManagement_);
+            snprintf(title,sizeof(title), DBT_serverAccountManagement_);
     }
+    title[sizeof(title)-1] = 0;
     fprintf(stdout,title);
 
     fprintf(stdout, "</B></FONT></FONT></FONT> </td></tr></table></TD></TR><TR> <TD>"
@@ -767,7 +778,7 @@
                                 "scrolling=auto marginwidth=0 marginheight=0 "
                                 "noresize border=0 "
                                 "name='%s'>\");\n", SCRIPT_NAME, TOP_NAME);
-	 GetFileForLanguage(HTML_FILE,GetClientLanguage(),existingpath);
+    GetFileForLanguage(HTML_FILE,GetClientLanguage(),existingpath,sizeof(existingpath));
 
     fprintf(stdout, "    top.bottom.document.writeln(\"<frame "
                                 "src='%s' "


Index: index.c
===================================================================
RCS file: /cvs/dirsec/adminserver/admserv/user-forms/src/index.c,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -r1.3 -r1.4
--- index.c	18 Aug 2005 19:10:57 -0000	1.3
+++ index.c	31 Mar 2006 22:58:21 -0000	1.4
@@ -86,7 +86,7 @@
     char *tmp=NULL, *data=NULL, *div=NULL;
     char *root = getenv("NETSITE_ROOT");
     
-    GetFileForLanguage(CATLIST_FILE,GetClientLanguage(),existingpath);
+    GetFileForLanguage(CATLIST_FILE,GetClientLanguage(),existingpath,sizeof(existingpath));
     f = fopen(existingpath, "r");
     if(!f)  {
         fprintf(stdout,
@@ -157,8 +157,9 @@
             if(!data) continue; /* huh?! */
             data++;
            
-            sprintf(path, INCLUDE_PATH, root, data);
-            GetFileForLanguage(path,GetClientLanguage(),existingpath);
+            snprintf(path, sizeof(path), INCLUDE_PATH, root, data);
+            path[sizeof(path)-1] = 0;
+            GetFileForLanguage(path,GetClientLanguage(),existingpath,sizeof(existingpath));
             if (!(flst = fopen(existingpath, "r"))) {
               fprintf(stdout,
                       XP_GetClientStr(DBT_errorCouldNotOpenSServerListFile_),
@@ -296,7 +297,7 @@
     fprintf(stdout, "    top.bottom.document.writeln(\"<frame src='%s?list' "
                                 "scrolling=no marginwidth=0 marginheight=0 "
                                 "name='%s'>\");\n", SCRIPT_NAME, TOP_NAME);
-    GetFileForLanguage(HTML_FILE,GetClientLanguage(),existingpath);
+    GetFileForLanguage(HTML_FILE,GetClientLanguage(),existingpath,sizeof(existingpath));
     fprintf(stdout, "    top.bottom.document.writeln(\"<frame "
                                 "src='%s' "
                                 "scrolling=yes marginwidth=4 "




More information about the Fedora-directory-commits mailing list