2005-10-02 Dmitry V. Levin <ldv@altlinux.org>

	    Steve Langasek  <vorlon@debian.org>

	Cleanup gratuitous use of strdup().
	Fix "missing argument" checks.

	* modules/pam_env/pam_env.c (_pam_parse): Add const qualifier
	to conffile and envfile arguments.  Do not use x_strdup() for
	conffile and envfile initialization.  Fix "missing argument"
	checks.
	(_parse_config_file): Take conffile argument of type "const char *"
	instead of "char **".  Do not free conffile.
	(_parse_env_file): Take env_file argument of type "const char *"
	instead of "char **".  Do not free env_file.
	(pam_sm_setcred): Add const qualifier to conf_file and env_file.
	Pass conf_file and env_file to _parse_config_file() and
	_parse_env_file() by value.
	(pam_sm_open_session): Likewise.

	* modules/pam_ftp/pam_ftp.c (_pam_parse): Add const qualifier to
	users argument.  Do not use x_strdup() for users initialization.
	(lookup):  Add const qualifier to list argument.
	(pam_sm_authenticate): Add const qualifier to users argument.

	* modules/pam_mail/pam_mail.c (_pam_parse): Add const qualifier
	to maildir argument.  Do not use x_strdup() for maildir
	initialization.  Fix "missing argument" check.
	(get_folder): Take path_mail argument of type "const char *"
	instead of "char **".  Do not free path_mail.
	(_do_mail): Add const qualifier to path_mail argument.
	Pass path_mail to get_folder() by value.

	* modules/pam_motd/pam_motd.c: Include <syslog.h>.
	(pam_sm_open_session): Add const qualifier to motd_path.
	Do not use x_strdup() for motd_path initialization.  Do not
	free motd_path.  Fix "missing argument" check.	Add "unknown
	option" warning.

	* modules/pam_userdb/pam_userdb.c (_pam_parse): Add const
	qualifier to database and cryptmode arguments.	Fix "missing
	argument" checks.
	(pam_sm_authenticate): Add const qualifier to database and cryptmode.
	(pam_sm_acct_mgmt): Likewise.

6 files changed, 111 insertions, 71 deletions

diff --git a/ChangeLog b/ChangeLog
index bfcafa77..263ff4c8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,47 @@
+2005-10-02  Dmitry V. Levin  <ldv@altlinux.org>
+	    Steve Langasek  <vorlon@debian.org>
+
+	Cleanup gratuitous use of strdup().
+	Fix "missing argument" checks.
+
+	* modules/pam_env/pam_env.c (_pam_parse): Add const qualifier
+	to conffile and envfile arguments.  Do not use x_strdup() for
+	conffile and envfile initialization.  Fix "missing argument"
+	checks.
+	(_parse_config_file): Take conffile argument of type "const char *"
+	instead of "char **".  Do not free conffile.
+	(_parse_env_file): Take env_file argument of type "const char *"
+	instead of "char **".  Do not free env_file.
+	(pam_sm_setcred): Add const qualifier to conf_file and env_file.
+	Pass conf_file and env_file to _parse_config_file() and
+	_parse_env_file() by value.
+	(pam_sm_open_session): Likewise.
+
+	* modules/pam_ftp/pam_ftp.c (_pam_parse): Add const qualifier to
+	users argument.  Do not use x_strdup() for users initialization.
+	(lookup):  Add const qualifier to list argument.
+	(pam_sm_authenticate): Add const qualifier to users argument.
+
+	* modules/pam_mail/pam_mail.c (_pam_parse): Add const qualifier
+	to maildir argument.  Do not use x_strdup() for maildir
+	initialization.  Fix "missing argument" check.
+	(get_folder): Take path_mail argument of type "const char *"
+	instead of "char **".  Do not free path_mail.
+	(_do_mail): Add const qualifier to path_mail argument.
+	Pass path_mail to get_folder() by value.
+
+	* modules/pam_motd/pam_motd.c: Include <syslog.h>.
+	(pam_sm_open_session): Add const qualifier to motd_path.
+	Do not use x_strdup() for motd_path initialization.  Do not
+	free motd_path.  Fix "missing argument" check.	Add "unknown
+	option" warning.
+
+	* modules/pam_userdb/pam_userdb.c (_pam_parse): Add const
+	qualifier to database and cryptmode arguments.	Fix "missing
+	argument" checks.
+	(pam_sm_authenticate): Add const qualifier to database and cryptmode.
+	(pam_sm_acct_mgmt): Likewise.
+
 2005-10-01  Steve Langasek  <vorlon@debian.org>
 
 	* modules/pam_userdb/pam_userdb.c: spelling fix in log message.
diff --git a/modules/pam_env/pam_env.c b/modules/pam_env/pam_env.c
index 9992bddb..f3098632 100644
--- a/modules/pam_env/pam_env.c
+++ b/modules/pam_env/pam_env.c
@@ -81,7 +81,7 @@ static char quote='Z';
 
 static int
 _pam_parse (const pam_handle_t *pamh, int argc, const char **argv,
-	    char **conffile, char **envfile, int *readenv)
+	    const char **conffile, const char **envfile, int *readenv)
 {
     int ctrl=0;
 
@@ -94,22 +94,22 @@ _pam_parse (const pam_handle_t *pamh, int argc, const char **argv,
 	if (!strcmp(*argv,"debug"))
 	    ctrl |= PAM_DEBUG_ARG;
 	else if (!strncmp(*argv,"conffile=",9)) {
-	    *conffile = x_strdup(9+*argv);
-	    if (*conffile != NULL) {
+	    *conffile = 9 + *argv;
+	    if (**conffile != '\0') {
 		D(("new Configuration File: %s", *conffile));
 		ctrl |= PAM_NEW_CONF_FILE;
 	    } else {
 		pam_syslog(pamh, LOG_ERR,
-			 "Configuration file specification missing argument - ignored");
+			 "conffile= specification missing argument - ignored");
 	    }
 	} else if (!strncmp(*argv,"envfile=",8)) {
-	    *envfile = x_strdup(8+*argv);
-	    if (*envfile != NULL) {
+	    *envfile = 8 + *argv;
+	    if (**envfile != '\0') {
 		D(("new Env File: %s", *envfile));
 		ctrl |= PAM_NEW_ENV_FILE;
 	    } else {
 		pam_syslog (pamh, LOG_ERR,
-			 "Env file specification missing argument - ignored");
+			 "envfile= specification missing argument - ignored");
 	    }
 	} else if (!strncmp(*argv,"readenv=",8))
 	    *readenv = atoi(8+*argv);
@@ -120,7 +120,8 @@ _pam_parse (const pam_handle_t *pamh, int argc, const char **argv,
     return ctrl;
 }
 
-static int _parse_config_file(pam_handle_t *pamh, int ctrl, char **conffile)
+static int
+_parse_config_file(pam_handle_t *pamh, int ctrl, const char *conffile)
 {
     int retval;
     const char *file;
@@ -132,7 +133,7 @@ static int _parse_config_file(pam_handle_t *pamh, int ctrl, char **conffile)
     D(("Called."));
 
     if (ctrl & PAM_NEW_CONF_FILE) {
-	file = *conffile;
+	file = conffile;
     } else {
 	file = DEFAULT_CONF_FILE;
     }
@@ -178,14 +179,12 @@ static int _parse_config_file(pam_handle_t *pamh, int ctrl, char **conffile)
     /* tidy up */
     _clean_var(var);        /* We could have got here prematurely,
 			     * this is safe though */
-    _pam_overwrite(*conffile);
-    _pam_drop(*conffile);
-    file = NULL;
     D(("Exit."));
     return (retval != 0 ? PAM_ABORT : PAM_SUCCESS);
 }
 
-static int _parse_env_file(pam_handle_t *pamh, int ctrl, char **env_file)
+static int
+_parse_env_file(pam_handle_t *pamh, int ctrl, const char *env_file)
 {
     int retval=PAM_SUCCESS, i, t;
     const char *file;
@@ -193,7 +192,7 @@ static int _parse_env_file(pam_handle_t *pamh, int ctrl, char **env_file)
     FILE *conf;
 
     if (ctrl & PAM_NEW_ENV_FILE)
-	file = *env_file;
+	file = env_file;
     else
 	file = DEFAULT_ETC_ENVFILE;
 
@@ -260,9 +259,6 @@ static int _parse_env_file(pam_handle_t *pamh, int ctrl, char **env_file)
     (void) fclose(conf);
 
     /* tidy up */
-    _pam_overwrite(*env_file);
-    _pam_drop(*env_file);
-    file = NULL;
     D(("Exit."));
     return (retval != 0 ? PAM_IGNORE : PAM_SUCCESS);
 }
@@ -742,7 +738,7 @@ pam_sm_setcred (pam_handle_t *pamh, int flags UNUSED,
 		int argc, const char **argv)
 {
   int retval, ctrl, readenv=DEFAULT_READ_ENVFILE;
-  char *conf_file=NULL, *env_file=NULL;
+  const char *conf_file = NULL, *env_file = NULL;
 
   /*
    * this module sets environment variables read in from a file
@@ -751,10 +747,10 @@ pam_sm_setcred (pam_handle_t *pamh, int flags UNUSED,
   D(("Called."));
   ctrl = _pam_parse(pamh, argc, argv, &conf_file, &env_file, &readenv);
 
-  retval = _parse_config_file(pamh, ctrl, &conf_file);
+  retval = _parse_config_file(pamh, ctrl, conf_file);
 
   if(readenv && retval == PAM_SUCCESS)
-    retval = _parse_env_file(pamh, ctrl, &env_file);
+    retval = _parse_env_file(pamh, ctrl, env_file);
 
   /* indicate success or failure */
 
@@ -766,7 +762,7 @@ PAM_EXTERN int
 pam_sm_acct_mgmt (pam_handle_t *pamh UNUSED, int flags UNUSED,
 		  int argc UNUSED, const char **argv UNUSED)
 {
-  pam_syslog (pamh, LOG_NOTICE, "pam_sm_acct_mgmt called inappropriatly");
+  pam_syslog (pamh, LOG_NOTICE, "pam_sm_acct_mgmt called inappropriately");
   return PAM_SERVICE_ERR;
 }
 
@@ -775,7 +771,7 @@ pam_sm_open_session (pam_handle_t *pamh, int flags UNUSED,
 		     int argc, const char **argv)
 {
   int retval, ctrl, readenv=DEFAULT_READ_ENVFILE;
-  char *conf_file=NULL, *env_file=NULL;
+  const char *conf_file = NULL, *env_file = NULL;
 
   /*
    * this module sets environment variables read in from a file
@@ -784,10 +780,10 @@ pam_sm_open_session (pam_handle_t *pamh, int flags UNUSED,
   D(("Called."));
   ctrl = _pam_parse(pamh, argc, argv, &conf_file, &env_file, &readenv);
 
-  retval = _parse_config_file(pamh, ctrl, &conf_file);
+  retval = _parse_config_file(pamh, ctrl, conf_file);
 
   if(readenv && retval == PAM_SUCCESS)
-    retval = _parse_env_file(pamh, ctrl, &env_file);
+    retval = _parse_env_file(pamh, ctrl, env_file);
 
   /* indicate success or failure */
 
@@ -807,7 +803,7 @@ PAM_EXTERN int
 pam_sm_chauthtok (pam_handle_t *pamh UNUSED, int flags UNUSED,
 		  int argc UNUSED, const char **argv UNUSED)
 {
-  pam_syslog (pamh, LOG_NOTICE, "pam_sm_chauthtok called inappropriatly");
+  pam_syslog (pamh, LOG_NOTICE, "pam_sm_chauthtok called inappropriately");
   return PAM_SERVICE_ERR;
 }
 
diff --git a/modules/pam_ftp/pam_ftp.c b/modules/pam_ftp/pam_ftp.c
index 37056767..4f4231c2 100644
--- a/modules/pam_ftp/pam_ftp.c
+++ b/modules/pam_ftp/pam_ftp.c
@@ -43,7 +43,7 @@
 #define PAM_NO_ANON         04
 
 static int
-_pam_parse(pam_handle_t *pamh, int argc, const char **argv, char **users)
+_pam_parse(pam_handle_t *pamh, int argc, const char **argv, const char **users)
 {
     int ctrl=0;
 
@@ -55,12 +55,7 @@ _pam_parse(pam_handle_t *pamh, int argc, const char **argv, char **users)
 	if (!strcmp(*argv,"debug"))
 	    ctrl |= PAM_DEBUG_ARG;
 	else if (!strncmp(*argv,"users=",6)) {
-	    *users = x_strdup(6+*argv);
-	    if (*users == NULL) {
-		ctrl |= PAM_NO_ANON;
-		pam_syslog(pamh, LOG_CRIT,
-			   "failed to duplicate user list - anon off");
-	    }
+	    *users = 6 + *argv;
 	} else if (!strcmp(*argv,"ignore")) {
 	    ctrl |= PAM_IGNORE_EMAIL;
 	} else {
@@ -76,23 +71,26 @@ _pam_parse(pam_handle_t *pamh, int argc, const char **argv, char **users)
  * return 1 if listed 0 if not.
  */
 
-static int lookup(const char *name, char *list, const char **_user)
+static int lookup(const char *name, const char *list, const char **_user)
 {
     int anon = 0;
 
     *_user = name;                 /* this is the default */
-    if (list) {
+    if (list && *list) {
 	const char *l;
-	char *x;
+	char *list_copy, *x;
 
-	x = list;
-	while ((l = strtok(x, ","))) {
+	list_copy = x_strdup(list);
+	x = list_copy;
+	while (list_copy && (l = strtok(x, ","))) {
 	    x = NULL;
 	    if (!strcmp(name, l)) {
 		*_user = list;
 		anon = 1;
 	    }
 	}
+	_pam_overwrite(list_copy);
+	_pam_drop(list_copy);
     } else {
 #define MAX_L 2
 	static const char *l[MAX_L] = { "ftp", "anonymous" };
@@ -118,7 +116,7 @@ pam_sm_authenticate (pam_handle_t *pamh, int flags UNUSED,
 {
     int retval, anon=0, ctrl;
     const char *user;
-    char *users=NULL;
+    const char *users = NULL;
 
     /*
      * this module checks if the user name is ftp or annonymous. If
diff --git a/modules/pam_mail/pam_mail.c b/modules/pam_mail/pam_mail.c
index caa58eae..c67a36c4 100644
--- a/modules/pam_mail/pam_mail.c
+++ b/modules/pam_mail/pam_mail.c
@@ -62,7 +62,7 @@
 
 static int
 _pam_parse (const pam_handle_t *pamh, int flags, int argc,
-	    const char **argv, char **maildir, size_t *hashcount)
+	    const char **argv, const char **maildir, size_t *hashcount)
 {
     int ctrl=0;
 
@@ -84,13 +84,13 @@ _pam_parse (const pam_handle_t *pamh, int flags, int argc,
 	else if (!strcmp(*argv,"standard"))
 	    ctrl |= PAM_STANDARD_MAIL | PAM_EMPTY_TOO;
 	else if (!strncmp(*argv,"dir=",4)) {
-	    *maildir = x_strdup(4+*argv);
-	    if (*maildir != NULL) {
+	    *maildir = 4 + *argv;
+	    if (**maildir != '\0') {
 		D(("new mail directory: %s", *maildir));
 		ctrl |= PAM_NEW_MAIL_DIR;
 	    } else {
-		pam_syslog (pamh, LOG_CRIT,
-			    "failed to duplicate mail directory - ignored");
+		pam_syslog(pamh, LOG_ERR,
+			   "dir= specification missing argument - ignored");
 	    }
 	} else if (!strncmp(*argv,"hash=",5)) {
 	    char *ep = NULL;
@@ -112,7 +112,7 @@ _pam_parse (const pam_handle_t *pamh, int flags, int argc,
     }
 
     if ((*hashcount != 0) && !(ctrl & PAM_NEW_MAIL_DIR)) {
-	*maildir = x_strdup(DEFAULT_MAIL_DIRECTORY);
+	*maildir = DEFAULT_MAIL_DIRECTORY;
 	ctrl |= PAM_NEW_MAIL_DIR;
     }
 
@@ -121,7 +121,7 @@ _pam_parse (const pam_handle_t *pamh, int flags, int argc,
 
 static int
 get_folder(pam_handle_t *pamh, int ctrl,
-	   char **path_mail, char **folder_p, size_t hashcount)
+	   const char *path_mail, char **folder_p, size_t hashcount)
 {
     int retval;
     const char *user, *path;
@@ -136,7 +136,7 @@ get_folder(pam_handle_t *pamh, int ctrl,
     }
 
     if (ctrl & PAM_NEW_MAIL_DIR) {
-	path = *path_mail;
+	path = path_mail;
 	if (*path == '~') {	/* support for $HOME delivery */
 	    pwd = pam_modutil_getpwnam(pamh, user);
 	    if (pwd == NULL) {
@@ -149,7 +149,7 @@ get_folder(pam_handle_t *pamh, int ctrl,
 	     */
 	    if (!*++path || (*path == '/' && !*++path)) {
 		pam_syslog(pamh, LOG_ERR,
-			   "badly formed mail path [%s]", *path_mail);
+			   "badly formed mail path [%s]", path_mail);
 		retval = PAM_SERVICE_ERR;
 		goto get_folder_cleanup;
 	    }
@@ -197,8 +197,6 @@ get_folder(pam_handle_t *pamh, int ctrl,
     /* tidy up */
 
   get_folder_cleanup:
-    _pam_overwrite(*path_mail);
-    _pam_drop(*path_mail);
     user = NULL;
     path = NULL;
 
@@ -361,8 +359,8 @@ static int _do_mail(pam_handle_t *pamh, int flags, int argc,
 {
     int retval, ctrl;
     size_t hashcount;
-    char *path_mail = NULL, *folder = NULL;
-    const char *type;
+    char *folder = NULL;
+    const char *path_mail = NULL, *type;
 
     /*
      * this module (un)sets the MAIL environment variable, and checks if
@@ -378,7 +376,7 @@ static int _do_mail(pam_handle_t *pamh, int flags, int argc,
 
     /* which folder? */
 
-    retval = get_folder(pamh, ctrl, &path_mail, &folder, hashcount);
+    retval = get_folder(pamh, ctrl, path_mail, &folder, hashcount);
     if (retval != PAM_SUCCESS) {
 	D(("failed to find folder"));
 	return retval;
diff --git a/modules/pam_motd/pam_motd.c b/modules/pam_motd/pam_motd.c
index fff76e07..ff9b1690 100644
--- a/modules/pam_motd/pam_motd.c
+++ b/modules/pam_motd/pam_motd.c
@@ -20,6 +20,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <pwd.h>
+#include <syslog.h>
 
 #include <security/_pam_macros.h>
 #include <security/pam_ext.h>
@@ -53,7 +54,7 @@ int pam_sm_open_session(pam_handle_t *pamh, int flags,
 {
     int retval = PAM_IGNORE;
     int fd;
-    char *motd_path = NULL;
+    const char *motd_path = NULL;
     char *mtmp = NULL;
 
     if (flags & PAM_SILENT) {
@@ -63,13 +64,17 @@ int pam_sm_open_session(pam_handle_t *pamh, int flags,
     for (; argc-- > 0; ++argv) {
         if (!strncmp(*argv,"motd=",5)) {
 
-            motd_path = (char *) strdup(5+*argv);
-            if (motd_path != NULL) {
+            motd_path = 5 + *argv;
+            if (*motd_path != '\0') {
                 D(("set motd path: %s", motd_path));
-            } else {
-                D(("failed to duplicate motd path - ignored"));
-            }
+	    } else {
+		motd_path = NULL;
+		pam_syslog(pamh, LOG_ERR,
+			   "motd= specification missing argument - ignored");
+	    }
 	}
+	else
+	    pam_syslog(pamh, LOG_ERR, "unknown option: %s", *argv);
     }
 
     if (motd_path == NULL)
@@ -102,9 +107,6 @@ int pam_sm_open_session(pam_handle_t *pamh, int flags,
     if (fd >= 0)
 	close(fd);
 
-    if (motd_path != default_motd)
-	free(motd_path);
-
      return retval;
 }
 
diff --git a/modules/pam_userdb/pam_userdb.c b/modules/pam_userdb/pam_userdb.c
index 590fb313..d47d58eb 100644
--- a/modules/pam_userdb/pam_userdb.c
+++ b/modules/pam_userdb/pam_userdb.c
@@ -85,7 +85,7 @@ obtain_authtok(pam_handle_t *pamh)
 
 static int
 _pam_parse (pam_handle_t *pamh, int argc, const char **argv,
-	    char **database, char **cryptmode)
+	    const char **database, const char **cryptmode)
 {
   int ctrl;
 
@@ -114,16 +114,18 @@ _pam_parse (pam_handle_t *pamh, int argc, const char **argv,
       else if (!strncasecmp(*argv,"db=", 3))
 	{
 	  *database = (*argv) + 3;
-	  if ((*database == NULL) || (strlen (*database) == 0))
+	  if (**database == '\0') {
+	    *database = NULL;
 	    pam_syslog(pamh, LOG_ERR,
-		       "could not parse argument \"%s\"", *argv);
+		       "db= specification missing argument - ignored");
+	  }
 	}
       else if (!strncasecmp(*argv,"crypt=", 6))
 	{
 	  *cryptmode = (*argv) + 6;
-	  if ((*cryptmode == NULL) || (strlen (*cryptmode) == 0))
+	  if (**cryptmode == '\0')
 	    pam_syslog(pamh, LOG_ERR,
-		       "could not parse argument \"%s\"", *argv);
+		       "crypt= specification missing argument - ignored");
 	}
       else
 	{
@@ -327,13 +329,13 @@ pam_sm_authenticate(pam_handle_t *pamh, int flags UNUSED,
 {
      const char *username;
      const void *password;
-     char *database = NULL;
-     char *cryptmode = NULL;
+     const char *database = NULL;
+     const char *cryptmode = NULL;
      int retval = PAM_AUTH_ERR, ctrl;
 
      /* parse arguments */
      ctrl = _pam_parse(pamh, argc, argv, &database, &cryptmode);
-     if ((database == NULL) || (strlen(database) == 0)) {
+     if (database == NULL) {
         pam_syslog(pamh, LOG_ERR, "can not get the database name");
         return PAM_SERVICE_ERR;
      }
@@ -422,8 +424,8 @@ pam_sm_acct_mgmt(pam_handle_t *pamh, int flags UNUSED,
 		 int argc, const char **argv)
 {
     const char *username;
-    char *database = NULL;
-    char *cryptmode = NULL;
+    const char *database = NULL;
+    const char *cryptmode = NULL;
     int retval = PAM_AUTH_ERR, ctrl;
 
     /* parse arguments */