From 5293ab9b6b35633182183ff2506ced7bbb641fa4 Mon Sep 17 00:00:00 2001
From: Nathan Kinder <nkinder@redhat.com>
Date: Apr 26 2011 22:07:16 +0000
Subject: Bug 699907 - (cov#10843) Use of uninitialized variable in logging code


The first time that logMsg is called, logEnabled and enableVerified will
both be 0. This will cause us to fill in the logfile buffer with the file
name and then check for the existence of the logfile. If the logfile does
not exist, we leave enable_verified set and log_enable unset. This will
make future calls to logMsg() just return at line 82.

If we were able to access the file fine for reading the first time logMsg()
is called at line 90, we will then go into the if condition at line 100
to set logfp. We are guaranteed that logfile is filled in at this point.
Further calls to logMsg() will have logfp set, so we no longer need logfile
to be filled in.

The problem here is that the call to fopen() at line 101 might fail, leaving
logfp NULL and log_enabled set to 1. The next time logMsg() is called, we
would call fopen again at line 101, but logfile would not be filled in.

To fix this, we should not set log_enabled to 1 unless we have successfully
opened the log file for writing.  I also moved the code around so we only
attempt to call fopen on the file if we have filled the filename buffer in.

---

diff --git a/admserv/cgi-src40/ugdsconfig.c b/admserv/cgi-src40/ugdsconfig.c
index 4a20903..9be5332 100644
--- a/admserv/cgi-src40/ugdsconfig.c
+++ b/admserv/cgi-src40/ugdsconfig.c
@@ -74,7 +74,7 @@ static char *nonull_value(char *str);
 /*
  * Logging function
  */
-static FILE * logfp;
+static FILE * logfp = NULL;
 static int log_enabled = 0, enable_verified=0;
 static void logMsg(char *format, ...) {
 	char logfile[512];
@@ -83,22 +83,28 @@ static void logMsg(char *format, ...) {
 
 	/* Automatically enable logging if <cgi-name>.dbg file exists in the logs directory */
 	if (!log_enabled && !enable_verified) {
-        const char *logdir = util_get_log_dir();
+		const char *logdir = util_get_log_dir();
 		enable_verified = 1;
-        if (util_is_dir_ok(logdir)) {
-            PR_snprintf(logfile, sizeof(logfile), "%s/ugdsconfig.dbg", logdir);
-            logfp = fopen(logfile, "r");
-            if (logfp == NULL) {
-                return;
-            }
-            log_enabled = 1;
-            fclose(logfp);
-            logfp=NULL;
-        }
-    }
-
-	if (logfp==NULL) {
-		logfp = fopen(logfile, "w");
+		if (util_is_dir_ok(logdir)) {
+			PR_snprintf(logfile, sizeof(logfile), "%s/ugdsconfig.dbg", logdir);
+
+			/* Attempt to optn the log for reading
+			 * to check if it exists. */
+			logfp = fopen(logfile, "r");
+			if (logfp == NULL) {
+				return;
+			}
+			fclose(logfp);
+
+			/* Attempt to open the file for writing. */
+			logfp = fopen(logfile, "w");
+
+			/* If we opened the log for writing, go
+			 * ahead and enable logging. */
+			if (logfp != NULL) {
+				log_enabled = 1;
+			}
+		}
 	}
 
 	if (logfp != NULL) {