Relevant BUGIDs:

Purpose of commit: cleanup

Commit summary:
---------------
Major cleanup with bug fixes (testing of possible NULL return values)
Patch by Dmitry V. Levin (ldv)

2 files changed, 136 insertions, 155 deletions

diff --git a/CHANGELOG b/CHANGELOG
index b30cfa57..e1e98c78 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -59,6 +59,7 @@ bug report - outstanding bugs are listed here:
 * libpam: Add pam_syslog function for unified syslog messages from
   PAM modules (kukuk).
 * pam_tally, pam_time, pam_userdb: use pam_syslog and pam_prompt (ldv)
+* pam_issue: major cleanup (ldv)
 
 0.80: Wed Jul 13 13:23:20 CEST 2005
 * pam_tally: test for NULL data before dereferencing them (t8m)
diff --git a/modules/pam_issue/pam_issue.c b/modules/pam_issue/pam_issue.c
index 0ff5e646..7a8a24d5 100644
--- a/modules/pam_issue/pam_issue.c
+++ b/modules/pam_issue/pam_issue.c
@@ -26,18 +26,19 @@
 #include <unistd.h>
 #include <sys/utsname.h>
 #include <utmp.h>
-#include <malloc.h>
 #include <time.h>
-
-#include <security/_pam_macros.h>
+#include <syslog.h>
 
 #define PAM_SM_AUTH
 
+#include <security/_pam_macros.h>
 #include <security/pam_modules.h>
+#include <security/pam_ext.h>
 
 static int _user_prompt_set = 0;
 
-static char *do_prompt (FILE *);
+static int read_issue_raw(pam_handle_t *pamh, FILE *fp, char **prompt);
+static int read_issue_quoted(pam_handle_t *pamh, FILE *fp, char **prompt);
 
 /* --- authentication management functions (only) --- */
 
@@ -45,31 +46,26 @@ PAM_EXTERN int
 pam_sm_authenticate (pam_handle_t *pamh, int flags UNUSED,
 		     int argc, const char **argv)
 {
-    int retval = PAM_SUCCESS;
-    FILE *fd;
+    int retval = PAM_SERVICE_ERR;
+    FILE *fp;
+    const char *issue_file = NULL;
     int parse_esc = 1;
-    char *prompt_tmp = NULL;
-    const void *cur_prompt = NULL;
-    struct stat st;
-    char *issue_file = NULL;
+    const void *item = NULL;
+    const char *cur_prompt;
+    char *issue_prompt = NULL;
 
    /* If we've already set the prompt, don't set it again */
     if(_user_prompt_set)
 	return PAM_IGNORE;
-    else
-       /* we set this here so if we fail below, we wont get further
-	  than this next time around (only one real failure) */
-	_user_prompt_set = 1;
+
+    /* We set this here so if we fail below, we wont get further
+       than this next time around (only one real failure) */
+    _user_prompt_set = 1;
 
     for ( ; argc-- > 0 ; ++argv ) {
 	if (!strncmp(*argv,"issue=",6)) {
-	    issue_file = (char *) strdup(6+*argv);
-	    if (issue_file != NULL) {
-		D(("set issue_file to: %s", issue_file));
-	    } else {
-		D(("failed to strdup issue_file - ignored"));
-		return PAM_IGNORE;
-	    }
+	    issue_file = 6 + *argv;
+	    D(("set issue_file to: %s", issue_file));
 	} else if (!strcmp(*argv,"noesc")) {
 	    parse_esc = 0;
 	    D(("turning off escape parsing by request"));
@@ -78,96 +74,50 @@ pam_sm_authenticate (pam_handle_t *pamh, int flags UNUSED,
     }
 
     if (issue_file == NULL)
-	issue_file = strdup("/etc/issue");
+	issue_file = "/etc/issue";
 
-    if ((fd = fopen(issue_file, "r")) != NULL) {
-	int tot_size = 0;
-
-	if (fstat(fileno(fd), &st) < 0) {
-	    fclose(fd);
-	    if (issue_file)
-	        free(issue_file);
-	    return PAM_IGNORE;
-	}
+    if ((fp = fopen(issue_file, "r")) == NULL) {
+	pam_syslog(pamh, LOG_ERR, "error opening %s: %m", issue_file);
+	return PAM_SERVICE_ERR;
+    }
 
-	retval = pam_get_item(pamh, PAM_USER_PROMPT, &cur_prompt);
-	if (retval != PAM_SUCCESS) {
-	    fclose(fd);
-	    if (issue_file)
-	        free(issue_file);
-	    return PAM_IGNORE;
-	}
-	if (cur_prompt == NULL) {
-	    cur_prompt = "";
-	}
+    if ((retval = pam_get_item(pamh, PAM_USER_PROMPT, &item)) != PAM_SUCCESS) {
+	fclose(fp);
+	return retval;
+    }
 
-	/* first read in the issue file */
+    cur_prompt = item;
+    if (cur_prompt == NULL)
+	cur_prompt = "";
 
-	if (parse_esc) {
-	    prompt_tmp = do_prompt(fd);
-	    if (prompt_tmp == NULL) {
-	        fclose(fd);
-	        if (issue_file)
-	           free(issue_file);
-		return PAM_IGNORE;
-	    }
-	} else {
-	    int count = 0;
-
-	    prompt_tmp = malloc(st.st_size + 1);
-	    if (prompt_tmp == NULL) {
-	        fclose(fd);
-	        if (issue_file)
-	           free(issue_file);
-		return PAM_IGNORE;
-	    }
-	    memset (prompt_tmp, '\0', st.st_size + 1);
-	    count = fread(prompt_tmp, 1, st.st_size, fd);
-	    if (count != st.st_size) {
-	        fclose(fd);
-		retval = PAM_IGNORE;
-		goto cleanup;
-	    }
-	    prompt_tmp[st.st_size] = '\0';
-	}
+    if (parse_esc)
+	retval = read_issue_quoted(pamh, fp, &issue_prompt);
+    else
+	retval = read_issue_raw(pamh, fp, &issue_prompt);
 
-	fclose(fd);
+    fclose(fp);
 
-	tot_size = strlen(prompt_tmp) + strlen(cur_prompt) + 1;
+    if (retval != PAM_SUCCESS)
+	goto out;
 
-       /*
-	* alloc some extra space for the original prompt
-	* and postpend it to the buffer
-	*/
-	{
-	    char *prompt_tmp_tmp = prompt_tmp;
+    {
+	size_t size = strlen(issue_prompt) + strlen(cur_prompt) + 1;
+	char *new_prompt = realloc(issue_prompt, size);
 
-	    prompt_tmp = realloc(prompt_tmp, tot_size + 1);
-	    if (prompt_tmp == NULL) {
-		prompt_tmp = prompt_tmp_tmp;
-		retval = PAM_IGNORE;
-		goto cleanup;
-	    }
+	if (new_prompt == NULL) {
+	    pam_syslog(pamh, LOG_ERR, "out of memory");
+	    retval = PAM_BUF_ERR;
+	    goto out;
 	}
-
-	strcpy(prompt_tmp+strlen(prompt_tmp), cur_prompt);
-
-	prompt_tmp[tot_size] = '\0';
-
-	retval = pam_set_item(pamh, PAM_USER_PROMPT,
-			      (const char *) prompt_tmp);
-
-    cleanup:
-	free(issue_file);
-	free(prompt_tmp);
-
-    } else {
-	D(("could not open issue_file: %s", issue_file));
-	free(issue_file);
-	return PAM_IGNORE;
+	issue_prompt = new_prompt;
     }
 
-    return retval;
+    strcat(issue_prompt, cur_prompt);
+    retval = pam_set_item(pamh, PAM_USER_PROMPT,
+			      (const void *) issue_prompt);
+  out:
+    _pam_drop(issue_prompt);
+    return (retval == PAM_SUCCESS) ? PAM_IGNORE : retval;
 }
 
 PAM_EXTERN int
@@ -177,56 +127,86 @@ pam_sm_setcred (pam_handle_t *pamh UNUSED, int flags UNUSED,
      return PAM_IGNORE;
 }
 
-static char *
-do_prompt (FILE *fd)
+static int
+read_issue_raw(pam_handle_t *pamh, FILE *fp, char **prompt)
+{
+    char *issue;
+    struct stat st;
+
+    *prompt = NULL;
+
+    if (fstat(fileno(fp), &st) < 0) {
+	pam_syslog(pamh, LOG_ERR, "stat error: %m");
+	return PAM_SERVICE_ERR;
+    }
+
+    if ((issue = malloc(st.st_size + 1)) == NULL) {
+	pam_syslog(pamh, LOG_ERR, "out of memory");
+	return PAM_BUF_ERR;
+    }
+
+    if (fread(issue, 1, st.st_size, fp) != st.st_size) {
+	pam_syslog(pamh, LOG_ERR, "read error: %m");
+	_pam_drop(issue);
+	return PAM_SERVICE_ERR;
+    }
+
+    issue[st.st_size] = '\0';
+    *prompt = issue;
+    return PAM_SUCCESS;
+}
+
+static int
+read_issue_quoted(pam_handle_t *pamh, FILE *fp, char **prompt)
 {
     int c;
     size_t size = 1024;
     char *issue;
-    char buf[1024];
     struct utsname uts;
 
-    if (fd == NULL)
-	return NULL;
+    *prompt = NULL;
 
-    issue = (char *)malloc(size);
-    if (issue == NULL)
-	return NULL;
+    if ((issue = malloc(size)) == NULL) {
+	pam_syslog(pamh, LOG_ERR, "out of memory");
+	return PAM_BUF_ERR;
+    }
 
-    issue[0] = '\0'; /* zero this, for strcat to work on first buf */
+    issue[0] = '\0';
     (void) uname(&uts);
 
-    while ((c = getc(fd)) != EOF) {
+    while ((c = getc(fp)) != EOF) {
+	char buf[1024];
+
+	buf[0] = '\0';
 	if (c == '\\') {
-	    c = getc(fd);
+	    if ((c = getc(fp)) == EOF)
+		break;
 	    switch (c) {
 	      case 's':
-		snprintf (buf, 1024, "%s", uts.sysname);
+		strncat(buf, uts.sysname, sizeof(buf) - 1);
 		break;
 	      case 'n':
-		snprintf (buf, 1024, "%s", uts.nodename);
+		strncat(buf, uts.nodename, sizeof(buf) - 1);
 		break;
 	      case 'r':
-		snprintf (buf, 1024, "%s", uts.release);
+		strncat(buf, uts.release, sizeof(buf) - 1);
 		break;
 	      case 'v':
-		snprintf (buf, 1024, "%s", uts.version);
+		strncat(buf, uts.version, sizeof(buf) - 1);
 		break;
 	      case 'm':
-		snprintf (buf, 1024, "%s", uts.machine);
+		strncat(buf, uts.machine, sizeof(buf) - 1);
 		break;
 	      case 'o':
 		{
 		    char domainname[256];
 
-		    if (getdomainname(domainname, sizeof(domainname)) == -1)
-		      domainname[0] = '\0';
-		    else
-		      domainname[sizeof(domainname)-1] = '\0';
-		    snprintf (buf, 1024, "%s", domainname);
+		    if (getdomainname(domainname, sizeof(domainname)) >= 0) {
+			domainname[sizeof(domainname)-1] = '\0';
+			strncat(buf, domainname, sizeof(buf) - 1);
+		    }
 		}
 		break;
-
 	      case 'd':
 	      case 't':
 		{
@@ -244,71 +224,71 @@ do_prompt (FILE *fd)
 		    tm = localtime(&now);
 
 		    if (c == 'd')
-			snprintf (buf, 1024, "%s %s %d  %d",
+			snprintf (buf, sizeof buf, "%s %s %d  %d",
 				weekday[tm->tm_wday], month[tm->tm_mon],
-				tm->tm_mday,
-				tm->tm_year + 1900);
+				tm->tm_mday, tm->tm_year + 1900);
 		    else
-			snprintf (buf, 1024, "%02d:%02d:%02d",
+			snprintf (buf, sizeof buf, "%02d:%02d:%02d",
 				tm->tm_hour, tm->tm_min, tm->tm_sec);
 		}
 		break;
 	      case 'l':
 		{
 		    char *ttyn = ttyname(1);
-		    if (!strncmp(ttyn, "/dev/", 5))
-			ttyn += 5;
-		    snprintf (buf, 1024, "%s", ttyn);
+		    if (ttyn) {
+			if (!strncmp(ttyn, "/dev/", 5))
+			    ttyn += 5;
+			strncat(buf, ttyn, sizeof(buf) - 1);
+		    }
 		}
 		break;
 	      case 'u':
 	      case 'U':
 		{
-		    int users = 0;
+		    unsigned int users = 0;
 		    struct utmp *ut;
 		    setutent();
-		    while ((ut = getutent()))
+		    while ((ut = getutent())) {
 			if (ut->ut_type == USER_PROCESS)
-			users++;
+			    ++users;
+		    }
 		    endutent();
 		    if (c == 'U')
-			snprintf (buf, 1024, "%d %s", users,
+			snprintf (buf, sizeof buf, "%u %s", users,
 			          (users == 1) ? "user" : "users");
 		    else
-			snprintf (buf, 1024, "%d", users);
+			snprintf (buf, sizeof buf, "%u", users);
 		    break;
 		}
 	      default:
 		buf[0] = c; buf[1] = '\0';
 	    }
-	    if ((strlen(issue) + strlen(buf)) < size + 1) {
-		char *old_issue = issue;
-
-		size += strlen(buf) + 1;
-		issue = (char *) realloc (issue, size);
-		if (issue == NULL) {
-		    free(old_issue);
-		    return NULL;
-		}
-	    }
-	    strcat(issue, buf);
 	} else {
 	    buf[0] = c; buf[1] = '\0';
-	    if ((strlen(issue) + strlen(buf)) < size + 1) {
-		char *old_issue = issue;
-
-		size += strlen(buf) + 1;
-		issue = (char *) realloc (issue, size);
-		if (issue == NULL) {
-		    free(old_issue);
-		    return NULL;
-		}
+	}
+
+	if ((strlen(issue) + strlen(buf)) + 1 > size) {
+	    char *new_issue;
+
+	    size += strlen(buf) + 1;
+	    new_issue = (char *) realloc (issue, size);
+	    if (new_issue == NULL) {
+		_pam_drop(issue);
+		return PAM_BUF_ERR;
 	    }
+	    issue = new_issue;
 	    strcat(issue, buf);
 	}
     }
 
-    return issue;
+    if (ferror(fp)) {
+	pam_syslog(pamh, LOG_ERR, "read error: %m");
+	_pam_drop(issue);
+	return PAM_SERVICE_ERR;
+    }
+
+    *prompt = issue;
+    return PAM_SUCCESS;
 }
 
 #ifdef PAM_STATIC