[Fedora-directory-commits] adminserver/lib/libadmin admconf.c, 1.5, 1.6 form_get.c, 1.5, 1.6 referer.c, 1.5, 1.6 template.c, 1.6, 1.7 util.c, 1.6, 1.7

[Doctor Conrad]

Author: rmeggins

Update of /cvs/dirsec/adminserver/lib/libadmin
In directory cvs-int.fedora.redhat.com:/tmp/cvs-serv28761/adminserver/lib/libadmin

Modified Files:
    admconf.c form_get.c referer.c template.c util.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: admconf.c
===================================================================
RCS file: /cvs/dirsec/adminserver/lib/libadmin/admconf.c,v
retrieving revision 1.5
retrieving revision 1.6
diff -u -r1.5 -r1.6
--- admconf.c	18 Aug 2005 19:20:01 -0000	1.5
+++ admconf.c	31 Mar 2006 22:58:29 -0000	1.6
@@ -93,7 +93,7 @@
    if (getenv("HTTP_REFERER")) strcpy(scratch, getenv("HTTP_REFERER")); else
 /* next sprintf is the 'else' part */
 #endif
-        sprintf(scratch, "%s%s", getenv("SERVER_URL"), getenv("SCRIPT_NAME"));
+        PR_snprintf(scratch, sizeof(scratch), "%s%s", getenv("SERVER_URL"), getenv("SCRIPT_NAME"));
         config[2] = STRDUP(scratch);
         config[3] = STRDUP(CONFIG3_DEF);
         config[4] = STRDUP(CONFIG4_DEF);
@@ -133,7 +133,7 @@

if(!fgets(scratch, 1024, f))
-            sprintf(scratch, "%s", CONFIG1_DEF);
+            PR_snprintf(scratch, sizeof(scratch), "%s", CONFIG1_DEF);
         else
             scratch[strlen(scratch)-1] = '\0';
         config[1] = STRDUP(scratch);
@@ -145,19 +145,19 @@
         config[2] = STRDUP(scratch);

if(!fgets(scratch, 1024, f))
-            sprintf(scratch, "%s", CONFIG3_DEF);
+            PR_snprintf(scratch, sizeof(scratch), "%s", CONFIG3_DEF);
         else
             scratch[strlen(scratch)-1] = '\0';
         config[3] = STRDUP(scratch);

if(!fgets(scratch, 1024, f))
-            sprintf(scratch, "%s", CONFIG4_DEF);
+            PR_snprintf(scratch, sizeof(scratch), "%s", CONFIG4_DEF);
         else
             scratch[strlen(scratch)-1] = '\0';
         config[4] = STRDUP(scratch);

if(!fgets(scratch, 1024, f))
-            sprintf(scratch, "%s", CONFIG5_DEF);
+            PR_snprintf(scratch, sizeof(scratch), "%s", CONFIG5_DEF);
         else
             scratch[strlen(scratch)-1] = '\0';
         {int n=0, x=0; for(x=0; scratch[x]; x++) if(scratch[x]==':') n++;

Index: form_get.c
===================================================================
RCS file: /cvs/dirsec/adminserver/lib/libadmin/form_get.c,v
retrieving revision 1.5
retrieving revision 1.6
diff -u -r1.5 -r1.6
--- form_get.c	18 Aug 2005 19:20:01 -0000	1.5
+++ form_get.c	31 Mar 2006 22:58:29 -0000	1.6
@@ -79,7 +79,7 @@

PR_snprintf(filePattern, sizeof(filePattern), "%s%s%s", HTML_DIR, "$$LANGDIR/", filename);

-    GetFileForLanguage(filePattern,language,line);
+    GetFileForLanguage(filePattern,language,line,sizeof(line));
     if(!(f = fopen(line, "r")))  {
         report_error(FILE_ERROR, line, "Could not open the HTML file.  "
                      "Perhaps the permissions have changed or someone "

Index: referer.c
===================================================================
RCS file: /cvs/dirsec/adminserver/lib/libadmin/referer.c,v
retrieving revision 1.5
retrieving revision 1.6
diff -u -r1.5 -r1.6
--- referer.c	18 Aug 2005 19:20:01 -0000	1.5
+++ referer.c	31 Mar 2006 22:58:29 -0000	1.6
@@ -131,9 +131,14 @@
 NSAPI_PUBLIC void redirect_to_script(char *script)
 {
     char urlbuf[BIG_LINE];
-
+    char *ptr;

PR_snprintf(urlbuf, sizeof(urlbuf), "%s%s", getenv("SERVER_URL"), getenv("SCRIPT_NAME"));
-    strcpy(strrchr(urlbuf, '/') + 1, script);
+    if (ptr = strrchr(urlbuf, '/')) {
+        int maxsize = sizeof(urlbuf)-((ptr-urlbuf)+2); /* one for the '/' and one for the '0' */
+        PL_strncpyz(ptr + 1, script, maxsize);
+    } else {
+        PR_snprintf(urlbuf, sizeof(urlbuf), "%s/%s", getenv("SERVER_URL"), script);
+    }
     printf("Location: %s\n\n", urlbuf);
 }

Index: template.c
===================================================================
RCS file: /cvs/dirsec/adminserver/lib/libadmin/template.c,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -r1.6 -r1.7
--- template.c	9 Sep 2005 19:04:01 -0000	1.6
+++ template.c	31 Mar 2006 22:58:29 -0000	1.7
@@ -397,7 +397,7 @@
     /*
      * URL changed to add new "mapfile" parameter for 5.0 help system - Adam
      */
-    util_snprintf( line, BIG_LINE,
+    util_snprintf( line, sizeof(line),
    	   "window.open('%s/manual/help/help?helpdir=admin&token=%s', '"
    	   INFO_IDX_NAME"_%s', "
    	   HELP_WIN_OPTIONS");",
@@ -427,7 +427,7 @@
     char outline[BIG_LINE];

if(verify)  {
-        util_snprintf(line, BIG_LINE, "<SCRIPT language="MOCHA_NAME">\n"
+        util_snprintf(line, sizeof(line), "<SCRIPT language="MOCHA_NAME">\n"
                       "function verify(form)  {\n"
                       "    if(confirm('Do you really want to %s?'))\n"
                       "        form.submit();\n"
@@ -439,14 +439,14 @@
     output("<center><table border=2 width=100%%><tr>");

if(!verify)  {
-        util_snprintf(outline, BIG_LINE, "%s%s%s%s%s",
+        util_snprintf(outline, sizeof(outline), "%s%s%s%s%s",
             "<td width=33%% align=center>",
             "<input type=submit value="",
             XP_GetAdminStr(DBT_ok_),
             "">",
             "</td>\n");
     }  else  {
-        util_snprintf(outline, BIG_LINE, "%s%s%s%s%s%s",
+        util_snprintf(outline, sizeof(outline), "%s%s%s%s%s%s",
             "<td width=33%% align=center>",
             "<input type=button value="",
             XP_GetAdminStr(DBT_ok_),
@@ -455,14 +455,14 @@
             "</td>\n");
     }
     output(outline);
-    util_snprintf(outline, BIG_LINE, "%s%s%s%s",
+    util_snprintf(outline, sizeof(outline), "%s%s%s%s",
         "<td width=34%% align=center>",
         "<input type=reset value="",
         XP_GetAdminStr(DBT_reset_),
         ""></td>\n");
     output(outline);

-    util_snprintf(line, BIG_LINE, "<td width=33%% align=center>%s</td>\n",
+    util_snprintf(line, sizeof(line), "<td width=33%% align=center>%s</td>\n",
                   _get_help_button( vars[0] ));
     output(line);

@@ -502,7 +502,7 @@
     /*
      * URL changed to add new "mapfile" parameter for 5.0 help system - Adam
      */
-    util_snprintf( line, BIG_LINE,
+    util_snprintf( line, sizeof(line),
    	  "<A HREF="javascript:"
    	  "if ( top.helpwin ) {"
    	  "  top.helpwin.focus();"
@@ -624,7 +624,7 @@
             }  else
                 isvar = 0;
         else
-            if(isvar != -1)  {
+            if((isvar != -1) && (isvar < sizeof(scratch)))  {
                 scratch[isvar++] = *string; 
                 scratch[isvar] = '\0';
             }

Index: util.c
===================================================================
RCS file: /cvs/dirsec/adminserver/lib/libadmin/util.c,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -r1.6 -r1.7
--- util.c	18 Aug 2005 19:20:01 -0000	1.6
+++ util.c	31 Mar 2006 22:58:29 -0000	1.7
@@ -193,7 +193,7 @@
    char *slash = NULL;

if (dir)
-      strcpy (path, dir);
+      PL_strncpyz (path, dir, sizeof(path));
    else
       return 0;

@@ -982,8 +982,8 @@
         }
         else {
             if (inet_addr(candidate) != -1) {
-                strcpy(work, candidate);
-                strcat(work, " 255.255.255.255");
+                PL_strncpyz(work, candidate, sizeof(work));
+                PL_strcatn(work, sizeof(work), " 255.255.255.255");
                 result = strdup(work);
             }
         }