[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

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;