[Fedora-directory-commits] adminserver/admserv/newinst/src ux-config.cc, 1.8, 1.9 ux-dialog.cc, 1.9, 1.10 ux-remove.cc, 1.4, 1.5

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/newinst/src
In directory cvs-int.fedora.redhat.com:/tmp/cvs-serv28761/adminserver/admserv/newinst/src

Modified Files:
	ux-config.cc ux-dialog.cc ux-remove.cc 
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: ux-config.cc
===================================================================
RCS file: /cvs/dirsec/adminserver/admserv/newinst/src/ux-config.cc,v
retrieving revision 1.8
retrieving revision 1.9
diff -u -r1.8 -r1.9
--- ux-config.cc	29 Mar 2006 02:19:52 -0000	1.8
+++ ux-config.cc	31 Mar 2006 22:58:20 -0000	1.9
@@ -158,7 +158,8 @@
          if (InstUtil::isServerRoot(_serverRoot) == False)
          {
             err = -1;
-            sprintf(errMsg, "ERROR: %s is not a server root\n",_serverRoot.data());
+            snprintf(errMsg, sizeof(errMsg), "ERROR: %s is not a server root\n",_serverRoot.data());
+            errMsg[sizeof(errMsg)-1] = 0;
          }
          else
          {
@@ -167,7 +168,8 @@
       }
       else if (InstUtil::fileExists(_infoFile) == False)
       {
-         sprintf(errMsg, "ERROR: answer cache not found\n");
+         snprintf(errMsg, sizeof(errMsg), "ERROR: answer cache not found\n");
+         errMsg[sizeof(errMsg)-1] = 0;
          err = -1;
       }
    }
@@ -176,7 +178,8 @@
       if (_infoFile == (char *) NULL ||
           InstUtil::fileExists(_infoFile) == False)
       {
-         sprintf(errMsg, "ERROR: installation script not found\n");
+         snprintf(errMsg, sizeof(errMsg), "ERROR: installation script not found\n");
+         errMsg[sizeof(errMsg)-1] = 0;
 	 _installLog->logMessage(FATAL, "Admin", "Installation script not found");
          err = -1;
       }
@@ -205,7 +208,8 @@
          {
             if (_adminInfo == NULL)
             {
-               sprintf(errMsg, "ERROR: Invalid installation script. No Admin section.\n");
+               snprintf(errMsg, sizeof(errMsg), "ERROR: Invalid installation script. No Admin section.\n");
+               errMsg[sizeof(errMsg)-1] = 0;
 	       _installLog->logMessage(FATAL, "Admin", "Invalid installation script. no Admin section");
                err = -1;
             }
@@ -267,7 +271,8 @@
    NSString hostName = InstUtil::guessHostname();
 
    /* First, determine whether Admin is already configured */
-   sprintf(tmp, "%s/admin-serv/config/adm.conf", _serverRoot.data());
+   snprintf(tmp, sizeof(tmp), "%s/admin-serv/config/adm.conf", _serverRoot.data());
+   tmp[sizeof(tmp)-1] = 0;
 
    admConf.setFormat(2);
    admConf.read(tmp);
@@ -303,7 +308,8 @@
       }
    }
 
-   sprintf(tmp, "%s/admin-serv/config/admpw", _serverRoot.data());
+   snprintf(tmp, sizeof(tmp), "%s/admin-serv/config/admpw", _serverRoot.data());
+   tmp[sizeof(tmp)-1] = 0;
 
    admConf.read(tmp);
 
@@ -688,13 +694,13 @@
 
    if (strncmp(errMsg, adminPort, 6) || port > MAXPORT)
    {
-      sprintf(errMsg, "OVERFLOW ERROR: Unable to bind to port %d\n"
+      snprintf(errMsg, sizeof(errMsg), "OVERFLOW ERROR: Unable to bind to port %d\n"
                  "Please choose another port less than %d.\n",
                    port, MAXPORT);
    }
    else if (InstUtil::portAvailable(port) == False)
    {
-      sprintf(errMsg,
+      snprintf(errMsg, sizeof(errMsg),
 "ERROR: Unable to bind to port %d\n"
 "       Please choose another port.\n",
         port);
@@ -703,6 +709,7 @@
    {
       errMsg[0] = 0;
    }
+   errMsg[sizeof(errMsg)-1] = 0;
    return errMsg;
 }
 
@@ -718,13 +725,13 @@
    {
       case -1:
 
-         sprintf(errMsg,
+         snprintf(errMsg, sizeof(errMsg),
 "The user %s does not currently exist.\n"
 "       Choose another, or create it an choose it again.\n",
         sysUser);
          break;
       case -2:
-         sprintf(errMsg,
+         snprintf(errMsg, sizeof(errMsg),
 "The system will not allow you to run the Administration\n"
 "       server as that user. Please choose another one.\n");
          break;
@@ -733,6 +740,7 @@
          break;
    }
 
+   errMsg[sizeof(errMsg)-1] = 0;
    return errMsg;
 }
 
@@ -750,7 +758,8 @@
    NSString errMsg;
    NSString sysUser;
 
-   sprintf(tmp, "%s/bin/admin/ns-admin", _serverRoot.data());
+   snprintf(tmp, sizeof(tmp), "%s/bin/admin/ns-admin", _serverRoot.data());
+   tmp[sizeof(tmp)-1] = 0;
 
    if (stat(tmp, &fi) == 0)
    {
@@ -778,14 +787,17 @@
    char stopProgram[BIG_BUF];
    struct stat fi;
 
-   sprintf(pid, "%s/admin-serv/logs/pid", _serverRoot.data());
+   snprintf(pid, sizeof(pid), "%s/admin-serv/logs/pid", _serverRoot.data());
+   pid[sizeof(pid)-1] = 0;
 
    if (stat(pid, &fi) == 0)
    {
-      sprintf(stopProgram, "%s/stop-admin", _serverRoot.data());
+       snprintf(stopProgram, sizeof(stopProgram), "%s/stop-admin", _serverRoot.data());
+       stopProgram[sizeof(stopProgram)-1] = 0;
       if (stat (stopProgram, &fi) != 0)
       {
-         sprintf(stopProgram, "kill `cat %s`", pid);
+         snprintf(stopProgram, sizeof(stopProgram), "kill `cat %s`", pid);
+         stopProgram[sizeof(stopProgram)-1] = 0;
       }
       system(stopProgram);
    }
@@ -793,7 +805,8 @@
    /* Stop SNMP Master Agent if running */
    int magpid;
    FILE* fhdl;
-   sprintf(pid, "%s/admin-serv/logs/pid_masteragt", _serverRoot.data());
+   snprintf(pid, sizeof(pid), "%s/admin-serv/logs/pid_masteragt", _serverRoot.data());
+   pid[sizeof(pid)-1] = 0;
    if ((fhdl = fopen(pid, "r")) != NULL)
    {
       fscanf(fhdl, "%d\n", &magpid);
@@ -811,7 +824,8 @@
 
    disableWinMode();
 
-   sprintf(tmp, "%s/bin/admin/ns-update -f %s", _serverRoot.data(), _infoFile.data());
+   snprintf(tmp, sizeof(tmp), "%s/bin/admin/ns-update -f %s", _serverRoot.data(), _infoFile.data());
+   tmp[sizeof(tmp)-1] = 0;
 
    err = system(tmp);
 
@@ -848,7 +862,8 @@
    char buf[1024];
    FILE *fp;
 
-   sprintf(cmd, "%s -V", file);
+   snprintf(cmd, sizeof(cmd), "%s -V", file);
+   cmd[sizeof(cmd)-1] = 0;
    fp = popen(cmd, "r");
 
    if (fp != NULL) {
@@ -880,10 +895,13 @@
    char *v;
 
    snprintf(path, sizeof(path), "%s/httpd.worker", dir);
+   path[sizeof(path)-1] = 0;
    if (stat(path, &st) != 0) {
       snprintf(path, sizeof(path), "%s/httpd", dir);
+      path[sizeof(path)-1] = 0;
       if (stat(path, &st) != 0) {
          snprintf(errMsg, sizeof(errMsg), "Can't find Apache in %s", dir);
+         errMsg[sizeof(errMsg)-1] = 0;
          return errMsg;
       }
    }
@@ -891,13 +909,16 @@
    v = get_value(path, "APACHE_MPM_DIR");
    if (strcmp(v, "server/mpm/worker")) {
       snprintf(errMsg, sizeof(errMsg), "The Admininistration Server requires an Apache web server that provides the worker model.");
+      errMsg[sizeof(errMsg)-1] = 0;
       return errMsg;
    }
 
    v = get_value(path, "HTTPD_ROOT");
-   sprintf(path, "%s/modules", v);
+   snprintf(path, sizeof(path), "%s/modules", v);
+   path[sizeof(path)-1] = 0;
    if (stat(path, &st) != 0) {
-      snprintf(errMsg, sizeof(errMsg), "Unable to locate Apache modules in %s\n.", path);;
+      snprintf(errMsg, sizeof(errMsg), "Unable to locate Apache modules in %s\n.", path);
+      errMsg[sizeof(errMsg)-1] = 0;
       return errMsg;
    }
 
@@ -915,15 +936,18 @@
    char *v;
 
    snprintf(path, sizeof(path), "%s/httpd.worker", dir);
+   path[sizeof(path)-1] = 0;
    if (stat(path, &st) != 0) {
       snprintf(path, sizeof(path), "%s/httpd", dir);
+      path[sizeof(path)-1] = 0;
       if (stat(path, &st) != 0) {
          return NULL;
       }
    }
 
    v = get_value(path, "HTTPD_ROOT");
-   sprintf(path, "%s/modules", v);
+   snprintf(path, sizeof(path), "%s/modules", v);
+   path[sizeof(path)-1] = 0;
    if (stat(path, &st) != 0) {
       return NULL;
    }


Index: ux-dialog.cc
===================================================================
RCS file: /cvs/dirsec/adminserver/admserv/newinst/src/ux-dialog.cc,v
retrieving revision 1.9
retrieving revision 1.10
diff -u -r1.9 -r1.10
--- ux-dialog.cc	28 Oct 2005 22:44:18 -0000	1.9
+++ ux-dialog.cc	31 Mar 2006 22:58:20 -0000	1.10
@@ -331,7 +331,6 @@
    const char *buf = me->input();
    int err = 0;
    char errMsg[BIG_BUF];
-   char tmp[XSM_BUF];
    LDAPURLDesc *ludpp;
    DialogAction rc = DIALOG_NEXT;
    AdminPreInstall *setup = ((AdminPreInstall *)me->manager()->parent());
@@ -354,17 +353,17 @@
         errMsg[0] = 0;
         break;
       case INVALID_URL:
-        sprintf(errMsg, "ERROR: The URL \"%s\" is not of valid format.\n", localLdapURL);
+        snprintf(errMsg, sizeof(errMsg), "ERROR: The URL \"%s\" is not of valid format.\n", localLdapURL);
         break;
       case CONN_FAILED:
-        sprintf(errMsg,
+        snprintf(errMsg, sizeof(errMsg),
 "ERROR: Cannot connect to URL \"%s\".\n"
 "       The server may have been down. Please fix the problem\n"
 "       before proceeding with installation.\n",
                 localLdapURL);
         break;
       case INVALID_DN:
-        sprintf(errMsg,
+        snprintf(errMsg, sizeof(errMsg),
 "ERROR: setup cannot verify the base suffix as specified in\n"
 "            \"%s\".\n"
 "       Please check the base suffix and re-enter the URL.\n",
@@ -377,6 +376,7 @@
 
    free(localLdapURL);
 
+   errMsg[sizeof(errMsg)-1] = 0;
    if (errMsg[0] != 0)
    {
       DialogAlert alert(errMsg);
@@ -410,7 +410,8 @@
 
    localLdapURL = UTF8ToLocal(installInfo->get(CONFIG_LDAP_URL));
 
-   sprintf(text2, "    %s", localLdapURL);
+   snprintf(text2, sizeof(text2), "    %s", localLdapURL);
+   text2[sizeof(text2)-1] = 0;
 
    me->setText2(text2);
 
@@ -477,25 +478,25 @@
        switch(err)
        {
           case INVALID_INPUT:
-             sprintf(errMsg, "ERROR: Invalid input.\n");
+             snprintf(errMsg, sizeof(errMsg), "ERROR: Invalid input.\n");
              break;
           case INVALID_URL:
-             sprintf(errMsg, "ERROR: Invalid URL \"%s\".\n", localLdapURL);
+             snprintf(errMsg, sizeof(errMsg), "ERROR: Invalid URL \"%s\".\n", localLdapURL);
              break;
           case INVALID_AUTH:
-             sprintf(errMsg, "ERROR: Insufficient authorization.\n");
+             snprintf(errMsg, sizeof(errMsg), "ERROR: Insufficient authorization.\n");
              break;
           case CONN_FAILED:
-             sprintf(errMsg,
+             snprintf(errMsg, sizeof(errMsg),
 "ERROR: Cannot connect to LDAP server.\n"
 "       The server may have been down. Please fix the problem\n"
 "       before proceeding with installation.\n");
              break;
           case INVALID_USER:
-             sprintf(errMsg, "ERROR: Invalid user and/or password.\n");
+             snprintf(errMsg, sizeof(errMsg), "ERROR: Invalid user and/or password.\n");
              break;
           default:
-             sprintf(errMsg,
+             snprintf(errMsg, sizeof(errMsg),
 "ERROR: Authentication failed. Either you have entered\n"
 "       an invalid user ID or password, or the directory server\n"
 "       is having some problem. Please check and re-enter.\n");
@@ -505,6 +506,7 @@
 
    free(localLdapURL);
 
+   errMsg[sizeof(errMsg)-1] = 0;
    if (errMsg [0] != 0)
    {
        DialogAlert alert(errMsg);


Index: ux-remove.cc
===================================================================
RCS file: /cvs/dirsec/adminserver/admserv/newinst/src/ux-remove.cc,v
retrieving revision 1.4
retrieving revision 1.5
diff -u -r1.4 -r1.5
--- ux-remove.cc	18 Aug 2005 19:06:43 -0000	1.4
+++ ux-remove.cc	31 Mar 2006 22:58:20 -0000	1.5
@@ -88,7 +88,8 @@
      serverRoot = uninstallInfo->get(SERVER_ROOT);
 
      instanceDir = serverRoot + "/" + "admin-serv";
-     sprintf(temp, "%s/admin-serv/config/adm.conf", serverRoot.data());
+     snprintf(temp, sizeof(temp), "%s/admin-serv/config/adm.conf", serverRoot.data());
+     temp[sizeof(temp)-1] = 0;
      admConf = new NVPair(temp);
 
      if (admConf->isEmpty() == False)
@@ -144,19 +145,23 @@
    FILE *fhdl = NULL;
 
    /* Stop the Admin Server */
-   sprintf(pidFile, "%s/admin-serv/logs/pid", serverRoot.data());
+   snprintf(pidFile, sizeof(pidFile), "%s/admin-serv/logs/pid", serverRoot.data());
+   pidFile[sizeof(pidFile)-1] = 0;
    if (stat(pidFile, &fi) == 0)
    {
-      sprintf(stopProgram, "%s/stop-admin", serverRoot.data());
+      snprintf(stopProgram, sizeof(stopProgram), "%s/stop-admin", serverRoot.data());
+      stopProgram[sizeof(stopProgram)-1] = 0;
       if (stat (stopProgram, &fi) != 0)
       {
-         sprintf(stopProgram, "kill `cat %s`", pid);
+         snprintf(stopProgram, sizeof(stopProgram), "kill `cat %s`", pid);
+         stopProgram[sizeof(stopProgram)-1] = 0;
       }
       system(stopProgram);
    }
 
    /* Stop SNMP Master Agent if running */
-   sprintf(pidFile, "%s/admin-serv/logs/pid_masteragt", serverRoot.data());
+   snprintf(pidFile, sizeof(pidFile), "%s/admin-serv/logs/pid_masteragt", serverRoot.data());
+   pidFile[sizeof(pidFile)-1] = 0;
    if ((fhdl = fopen(pidFile, "r")) != NULL)
    {
       fscanf(fhdl, "%d\n", &pid);
@@ -171,27 +176,35 @@
    szHomeDir = getenv("home");
    if(szHomeDir != NULL)
    {
-      sprintf(szCmdBuf, "rm -fr %s/.mcc", szHomeDir);
+      snprintf(szCmdBuf, sizeof(szCmdBuf), "rm -fr %s/.mcc", szHomeDir);
+      szCmdBuf[sizeof(szCmdBuf)-1] = 0;
+      
       system(szCmdBuf);
    }
 
    /* Remove dynamic and temporary files */
-   sprintf(szCmdBuf, "rm -fr %s/admin-serv/tmp", serverRoot.data());
+   snprintf(szCmdBuf, sizeof(szCmdBuf), "rm -fr %s/admin-serv/tmp", serverRoot.data());
+   szCmdBuf[sizeof(szCmdBuf)-1] = 0;
    system(szCmdBuf);
     
-   sprintf(szCmdBuf, "rm -fr %s/admin-serv/ClassCache", serverRoot.data());
+   snprintf(szCmdBuf, sizeof(szCmdBuf), "rm -fr %s/admin-serv/ClassCache", serverRoot.data());
+   szCmdBuf[sizeof(szCmdBuf)-1] = 0;
    system(szCmdBuf);
     
-   sprintf(szCmdBuf, "rm -fr %s/admin-serv/SessionData", serverRoot.data());
+   snprintf(szCmdBuf, sizeof(szCmdBuf), "rm -fr %s/admin-serv/SessionData", serverRoot.data());
+   szCmdBuf[sizeof(szCmdBuf)-1] = 0;
    system(szCmdBuf);
     
-   sprintf(szCmdBuf, "rm -fr %s/admin-serv/config/*.properties", serverRoot.data());
+   snprintf(szCmdBuf, sizeof(szCmdBuf), "rm -fr %s/admin-serv/config/*.properties", serverRoot.data());
+   szCmdBuf[sizeof(szCmdBuf)-1] = 0;
    system(szCmdBuf);
 
-   sprintf(szCmdBuf, "rm -fr %s/java/jars/*.icon", serverRoot.data());
+   snprintf(szCmdBuf, sizeof(szCmdBuf), "rm -fr %s/java/jars/*.icon", serverRoot.data());
+   szCmdBuf[sizeof(szCmdBuf)-1] = 0;
    system(szCmdBuf);
 
-   sprintf(szCmdBuf, "rm -fr %s/java/.java", serverRoot.data());
+   snprintf(szCmdBuf, sizeof(szCmdBuf), "rm -fr %s/java/.java", serverRoot.data());
+   szCmdBuf[sizeof(szCmdBuf)-1] = 0;
    system(szCmdBuf);
 
    return 0;

More information about the Fedora-directory-commits mailing list