Bug(s) fixed: 186280

Bug Description: ldapserver: Close potential security vulnerabilities in CGI code
Reviewed by: Nathan, Noriko, and Pete (Thanks!)
Fix Description: Clean up usage of sprintf, strcpy, fgets instead of
gets, fixed buffer usage, etc., mostly in the CGI code and other user
facing code (i.e. setup).  Also, Steve Grubb told me about a GCC trick
to force it to check printf style varargs functions, to check the format
string against the argument string, for type mismatches, missing
arguments, and too many arguments.
In the CGI form argument parsing code, we needed to be more careful
about checking for bad input - good input is supposed to look like this:
name=value&name=value&.....
&name=value.  I don't think the original code
was checking properly for something like name&name=value.
There was another place where we were not checking to see if a buffer
had enough room before appending a string to it.
I had to change a couple of functions to allow passing in the size of
the buffer.
Fixed some issues raised by Noriko and Nathan.
Platforms tested: RHEL4
Flag Day: no
Doc impact: no
QA impact: should be covered by regular nightly and manual testing
New Tests integrated into TET: none

5 files changed, 8 insertions, 23 deletions

diff --git a/lib/base/dns.cpp b/lib/base/dns.cpp
index 834dcd7f..f2e5454e 100644
--- a/lib/base/dns.cpp
+++ b/lib/base/dns.cpp
@@ -109,7 +109,9 @@ char *dns_ip2host(char *ip, int verify)
 
     err = PR_InitializeNetAddr(PR_IpAddrNull, 0, &iaddr);
 
-    if((iaddr.inet.ip = inet_addr(ip)) == -1)
+	/* richm: ipv6 cleanup - use inet_aton or other more appropriate function
+	   instead of inet_addr */
+    if((iaddr.inet.ip = inet_addr(ip)) == (in_addr_t)-1)
         goto bong;
 
     /*
diff --git a/lib/base/dnsdmain.cpp b/lib/base/dnsdmain.cpp
index 8a2ba133..6ed82428 100644
--- a/lib/base/dnsdmain.cpp
+++ b/lib/base/dnsdmain.cpp
@@ -177,7 +177,7 @@ extern "C" NSAPI_PUBLIC char *dns_guess_domain(char * hname)
 
     if (domain != 0) {
 	hnlen = strlen(hname);
-	if ((hnlen + dnlen + 2) <= sizeof(line)) {
+	if ((size_t)(hnlen + dnlen + 2) <= sizeof(line)) {
 	    strcpy(line, hname);
 	    line[hnlen] = '.';
 	    strcpy(&line[hnlen+1], domain);
diff --git a/lib/base/file.cpp b/lib/base/file.cpp
index 7c6028c7..46e9ba16 100644
--- a/lib/base/file.cpp
+++ b/lib/base/file.cpp
@@ -543,23 +543,6 @@ static char errmsg[ERRMSG_SIZE];
 
 #include "util.h"
 
-static char *_errmsg_new(int code)
-{
-    char *ret;
-#ifdef THREAD_ANY
-    if(!(ret = (char *) systhread_getdata(errmsg_key))) {
-        ret = (char *) PERM_MALLOC(256);
-        systhread_setdata(errmsg_key, (void *)ret);
-    }
-#else
-    ret = errmsg;
-#endif
-    util_snprintf(ret, ERRMSG_SIZE, "libsec code %d", code);
-#ifndef MCC_BATMAN
-    PR_SetError(0,0);
-#endif
-    return ret;
-}
 #endif
 
 
@@ -611,7 +594,7 @@ NSAPI_PUBLIC int system_errmsg_fn(char **buff, size_t maxlen)
             PR_SetError(0, 0);
             lmsg = nscp_error_msg;
         } else {
-            util_snprintf(static_error, ERRMSG_SIZE, "unknown error %d", nscp_error);
+            util_snprintf(static_error, sizeof(static_error), "unknown error %d", nscp_error);
             lmsg = static_error;
         }
     } else {
diff --git a/lib/base/plist.cpp b/lib/base/plist.cpp
index c2062767..6482cd05 100644
--- a/lib/base/plist.cpp
+++ b/lib/base/plist.cpp
@@ -818,7 +818,7 @@ PListNameProp(PList_t plist, int pindex, const char *pname)
 
                     /* Is it time to grow the hash table? */
                     i = PLSIZENDX(pt->pt_sizendx);
-                    if ((pt->pt_sizendx < PLMAXSIZENDX) &&
+                    if (((size_t)pt->pt_sizendx < PLMAXSIZENDX) &&
                         pt->pt_nsyms >= (i + i)) {
 
                         PLSymbolTable_t *npt;
diff --git a/lib/base/pool.cpp b/lib/base/pool.cpp
index 915199c7..8a379549 100644
--- a/lib/base/pool.cpp
+++ b/lib/base/pool.cpp
@@ -195,7 +195,7 @@ _free_block(block_t *block)
 	memset(block->data, 0xa, block->end-block->data);
 #endif /* POOL_ZERO_DEBUG */
 
-	if ((freelist_size + block->end - block->data) > freelist_max) {
+	if ((unsigned long)(freelist_size + block->end - block->data) > freelist_max) {
 		/* Just have to delete the whole block! */
 
 		crit_enter(freelist_lock);
@@ -452,7 +452,7 @@ pool_realloc(pool_handle_t *pool_handle, void *ptr, size_t size)
 	pool_t *pool = (pool_t *)pool_handle;
 	void *newptr;
 	block_t *block_ptr;
-	int oldsize;
+	size_t oldsize;
 
 	if (pool_handle == NULL || pool_disable)
 		return PERM_REALLOC(ptr, size);