[PATCH] Fix gratuitous use of strdup and x_strdup
by Dmitry V. Levin
There is no need to copy strings passed as arguments to execve,
the only potentially noticeable effect of using strdup/x_strdup
would be a malformed argument list in case of memory allocation error.
Also, x_strdup, being a thin wrapper around strdup, is of no benefit
when its argument is known to be non-NULL, and should not be used in
such cases.
* modules/pam_cracklib/pam_cracklib.c (password_check): Use strdup
instead of x_strdup, the latter is of no benefit in this case.
* modules/pam_ftp/pam_ftp.c (lookup): Likewise.
* modules/pam_userdb/pam_userdb.c (user_lookup): Likewise.
* modules/pam_userdb/pam_userdb.h (x_strdup): Remove.
* modules/pam_mkhomedir/pam_mkhomedir.c (create_homedir): Do not use
x_strdup for strings passed as arguments to execve.
* modules/pam_unix/pam_unix_acct.c (_unix_run_verify_binary): Likewise.
* modules/pam_unix/pam_unix_passwd.c (_unix_run_update_binary): Likewise.
* modules/pam_unix/support.c (_unix_run_helper_binary): Likewise.
(_unix_verify_password): Use strdup instead of x_strdup, the latter
is of no benefit in this case.
* modules/pam_xauth/pam_xauth.c (run_coprocess): Do not use strdup for
strings passed as arguments to execv.
---
modules/pam_cracklib/pam_cracklib.c | 6 +++---
modules/pam_ftp/pam_ftp.c | 2 +-
modules/pam_mkhomedir/pam_mkhomedir.c | 12 ++++++------
modules/pam_unix/pam_unix_acct.c | 10 +++++-----
modules/pam_unix/pam_unix_passwd.c | 16 ++++++++--------
modules/pam_unix/support.c | 16 ++++++++--------
modules/pam_userdb/pam_userdb.c | 2 +-
modules/pam_userdb/pam_userdb.h | 3 ---
modules/pam_xauth/pam_xauth.c | 12 +++++-------
9 files changed, 37 insertions(+), 42 deletions(-)
diff --git a/modules/pam_cracklib/pam_cracklib.c b/modules/pam_cracklib/pam_cracklib.c
index 5691347..5eefd0b 100644
--- a/modules/pam_cracklib/pam_cracklib.c
+++ b/modules/pam_cracklib/pam_cracklib.c
@@ -619,16 +619,16 @@ static const char *password_check(pam_handle_t *pamh, struct cracklib_options *o
return msg;
}
- newmono = str_lower(x_strdup(new));
+ newmono = str_lower(strdup(new));
if (!newmono)
msg = _("memory allocation error");
- usermono = str_lower(x_strdup(user));
+ usermono = str_lower(strdup(user));
if (!usermono)
msg = _("memory allocation error");
if (!msg && old) {
- oldmono = str_lower(x_strdup(old));
+ oldmono = str_lower(strdup(old));
if (oldmono)
wrapped = malloc(strlen(oldmono) * 2 + 1);
if (wrapped) {
diff --git a/modules/pam_ftp/pam_ftp.c b/modules/pam_ftp/pam_ftp.c
index 896a1dd..221d8f8 100644
--- a/modules/pam_ftp/pam_ftp.c
+++ b/modules/pam_ftp/pam_ftp.c
@@ -81,7 +81,7 @@ static int lookup(const char *name, const char *list, const char **_user)
char *list_copy, *x;
char *sptr = NULL;
- list_copy = x_strdup(list);
+ list_copy = strdup(list);
x = list_copy;
while (list_copy && (l = strtok_r(x, ",", &sptr))) {
x = NULL;
diff --git a/modules/pam_mkhomedir/pam_mkhomedir.c b/modules/pam_mkhomedir/pam_mkhomedir.c
index 0b5fc75..a867a73 100644
--- a/modules/pam_mkhomedir/pam_mkhomedir.c
+++ b/modules/pam_mkhomedir/pam_mkhomedir.c
@@ -134,7 +134,7 @@ create_homedir (pam_handle_t *pamh, options_t *opt,
int i;
struct rlimit rlim;
static char *envp[] = { NULL };
- char *args[] = { NULL, NULL, NULL, NULL, NULL };
+ const char *args[] = { NULL, NULL, NULL, NULL, NULL };
if (getrlimit(RLIMIT_NOFILE, &rlim)==0) {
if (rlim.rlim_max >= MAX_FD_NO)
@@ -145,12 +145,12 @@ create_homedir (pam_handle_t *pamh, options_t *opt,
}
/* exec the mkhomedir helper */
- args[0] = x_strdup(MKHOMEDIR_HELPER);
- args[1] = (char *) user;
- args[2] = x_strdup(opt->umask);
- args[3] = x_strdup(opt->skeldir);
+ args[0] = MKHOMEDIR_HELPER;
+ args[1] = user;
+ args[2] = opt->umask;
+ args[3] = opt->skeldir;
- execve(MKHOMEDIR_HELPER, args, envp);
+ execve(MKHOMEDIR_HELPER, (char *const *) args, envp);
/* should not get here: exit with error */
D(("helper binary is not available"));
diff --git a/modules/pam_unix/pam_unix_acct.c b/modules/pam_unix/pam_unix_acct.c
index 8ec4449..dc505e7 100644
--- a/modules/pam_unix/pam_unix_acct.c
+++ b/modules/pam_unix/pam_unix_acct.c
@@ -101,7 +101,7 @@ int _unix_run_verify_binary(pam_handle_t *pamh, unsigned int ctrl,
int i=0;
struct rlimit rlim;
static char *envp[] = { NULL };
- char *args[] = { NULL, NULL, NULL, NULL };
+ const char *args[] = { NULL, NULL, NULL, NULL };
/* reopen stdout as pipe */
dup2(fds[1], STDOUT_FILENO);
@@ -130,11 +130,11 @@ int _unix_run_verify_binary(pam_handle_t *pamh, unsigned int ctrl,
}
/* exec binary helper */
- args[0] = x_strdup(CHKPWD_HELPER);
- args[1] = x_strdup(user);
- args[2] = x_strdup("chkexpiry");
+ args[0] = CHKPWD_HELPER;
+ args[1] = user;
+ args[2] = "chkexpiry";
- execve(CHKPWD_HELPER, args, envp);
+ execve(CHKPWD_HELPER, (char *const *) args, envp);
pam_syslog(pamh, LOG_ERR, "helper binary execve failed: %m");
/* should not get here: exit with error */
diff --git a/modules/pam_unix/pam_unix_passwd.c b/modules/pam_unix/pam_unix_passwd.c
index 0cfc0f4..5f3a3db 100644
--- a/modules/pam_unix/pam_unix_passwd.c
+++ b/modules/pam_unix/pam_unix_passwd.c
@@ -204,7 +204,7 @@ static int _unix_run_update_binary(pam_handle_t *pamh, unsigned int ctrl, const
int i=0;
struct rlimit rlim;
static char *envp[] = { NULL };
- char *args[] = { NULL, NULL, NULL, NULL, NULL, NULL };
+ const char *args[] = { NULL, NULL, NULL, NULL, NULL, NULL };
char buffer[16];
/* XXX - should really tidy up PAM here too */
@@ -222,18 +222,18 @@ static int _unix_run_update_binary(pam_handle_t *pamh, unsigned int ctrl, const
}
/* exec binary helper */
- args[0] = x_strdup(UPDATE_HELPER);
- args[1] = x_strdup(user);
- args[2] = x_strdup("update");
+ args[0] = UPDATE_HELPER;
+ args[1] = user;
+ args[2] = "update";
if (on(UNIX_SHADOW, ctrl))
- args[3] = x_strdup("1");
+ args[3] = "1";
else
- args[3] = x_strdup("0");
+ args[3] = "0";
snprintf(buffer, sizeof(buffer), "%d", remember);
- args[4] = x_strdup(buffer);
+ args[4] = buffer;
- execve(UPDATE_HELPER, args, envp);
+ execve(UPDATE_HELPER, (char *const *) args, envp);
/* should not get here: exit with error */
D(("helper binary is not available"));
diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c
index 19d72e6..3a849c8 100644
--- a/modules/pam_unix/support.c
+++ b/modules/pam_unix/support.c
@@ -567,7 +567,7 @@ static int _unix_run_helper_binary(pam_handle_t *pamh, const char *passwd,
int i=0;
struct rlimit rlim;
static char *envp[] = { NULL };
- char *args[] = { NULL, NULL, NULL, NULL };
+ const char *args[] = { NULL, NULL, NULL, NULL };
/* XXX - should really tidy up PAM here too */
@@ -593,15 +593,15 @@ static int _unix_run_helper_binary(pam_handle_t *pamh, const char *passwd,
}
/* exec binary helper */
- args[0] = strdup(CHKPWD_HELPER);
- args[1] = x_strdup(user);
+ args[0] = CHKPWD_HELPER;
+ args[1] = user;
if (off(UNIX__NONULL, ctrl)) { /* this means we've succeeded */
- args[2]=strdup("nullok");
+ args[2]="nullok";
} else {
- args[2]=strdup("nonull");
+ args[2]="nonull";
}
- execve(CHKPWD_HELPER, args, envp);
+ execve(CHKPWD_HELPER, (char *const *) args, envp);
/* should not get here: exit with error */
D(("helper binary is not available"));
@@ -788,10 +788,10 @@ int _unix_verify_password(pam_handle_t * pamh, const char *name
login_name = "";
}
- new->user = x_strdup(name ? name : "");
+ new->user = strdup(name ? name : "");
new->uid = getuid();
new->euid = geteuid();
- new->name = x_strdup(login_name);
+ new->name = strdup(login_name);
/* any previous failures for this user ? */
if (pam_get_data(pamh, data_name, &void_old)
diff --git a/modules/pam_userdb/pam_userdb.c b/modules/pam_userdb/pam_userdb.c
index ff040e6..ba36ebf 100644
--- a/modules/pam_userdb/pam_userdb.c
+++ b/modules/pam_userdb/pam_userdb.c
@@ -184,7 +184,7 @@ user_lookup (pam_handle_t *pamh, const char *database, const char *cryptmode,
else
key.dsize = strlen(key.dptr);
} else {
- key.dptr = x_strdup(user);
+ key.dptr = strdup(user);
key.dsize = strlen(user);
}
diff --git a/modules/pam_userdb/pam_userdb.h b/modules/pam_userdb/pam_userdb.h
index 3cd8fee..86e9b47 100644
--- a/modules/pam_userdb/pam_userdb.h
+++ b/modules/pam_userdb/pam_userdb.h
@@ -15,9 +15,6 @@
#define PAM_USE_FPASS_ARG 0x0040
#define PAM_TRY_FPASS_ARG 0x0080
-/* Useful macros */
-#define x_strdup(s) ( (s) ? strdup(s):NULL )
-
/* The name of the module we are compiling */
#ifndef MODULE_NAME
#define MODULE_NAME "pam_userdb"
diff --git a/modules/pam_xauth/pam_xauth.c b/modules/pam_xauth/pam_xauth.c
index 32adcf6..c7ce55a 100644
--- a/modules/pam_xauth/pam_xauth.c
+++ b/modules/pam_xauth/pam_xauth.c
@@ -127,8 +127,7 @@ run_coprocess(pam_handle_t *pamh, const char *input, char **output,
if (child == 0) {
/* We're the child. */
size_t j;
- char *args[10];
- const char *tmp;
+ const char *args[10];
int maxopened;
/* Drop privileges. */
if (setgid(gid) == -1)
@@ -166,16 +165,15 @@ run_coprocess(pam_handle_t *pamh, const char *input, char **output,
}
/* Convert the varargs list into a regular array of strings. */
va_start(ap, command);
- args[0] = strdup(command);
+ args[0] = command;
for (j = 1; j < ((sizeof(args) / sizeof(args[0])) - 1); j++) {
- tmp = va_arg(ap, const char*);
- if (tmp == NULL) {
+ args[j] = va_arg(ap, const char*);
+ if (args[j] == NULL) {
break;
}
- args[j] = strdup(tmp);
}
/* Run the command. */
- execv(command, args);
+ execv(command, (char *const *) args);
/* Never reached. */
_exit(1);
}
--
ldv
10 years, 3 months
[PATCH] pam_xauth: log fatal errors preventing xauth process execution
by Dmitry V. Levin
* modules/pam_xauth/pam_xauth.c (run_coprocess): Log errors from pipe()
and fork() calls.
---
modules/pam_xauth/pam_xauth.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/modules/pam_xauth/pam_xauth.c b/modules/pam_xauth/pam_xauth.c
index 2ee1e8f..32adcf6 100644
--- a/modules/pam_xauth/pam_xauth.c
+++ b/modules/pam_xauth/pam_xauth.c
@@ -103,9 +103,11 @@ run_coprocess(pam_handle_t *pamh, const char *input, char **output,
/* Create stdio pipery. */
if (pipe(ipipe) == -1) {
+ pam_syslog(pamh, LOG_ERR, "Could not create pipe: %m");
return -1;
}
if (pipe(opipe) == -1) {
+ pam_syslog(pamh, LOG_ERR, "Could not create pipe: %m");
close(ipipe[0]);
close(ipipe[1]);
return -1;
@@ -114,6 +116,7 @@ run_coprocess(pam_handle_t *pamh, const char *input, char **output,
/* Fork off a child. */
child = fork();
if (child == -1) {
+ pam_syslog(pamh, LOG_ERR, "Could not fork: %m");
close(ipipe[0]);
close(ipipe[1]);
close(opipe[0]);
--
ldv
10 years, 3 months
[PATCH] pam_xauth: avoid potential SIGPIPE when writing to xauth process
by Dmitry V. Levin
Similar issue in pam_unix was fixed by commit Linux-PAM-0-73~8.
* modules/pam_xauth/pam_xauth.c (run_coprocess): In the parent process,
close the read end of input pipe after writing to its write end.
---
modules/pam_xauth/pam_xauth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/modules/pam_xauth/pam_xauth.c b/modules/pam_xauth/pam_xauth.c
index 88624b1..2ee1e8f 100644
--- a/modules/pam_xauth/pam_xauth.c
+++ b/modules/pam_xauth/pam_xauth.c
@@ -178,12 +178,12 @@ run_coprocess(pam_handle_t *pamh, const char *input, char **output,
}
/* We're the parent, so close the other ends of the pipes. */
- close(ipipe[0]);
close(opipe[1]);
/* Send input to the process (if we have any), then send an EOF. */
if (input) {
(void)pam_modutil_write(ipipe[1], input, strlen(input));
}
+ close(ipipe[0]); /* close here to avoid possible SIGPIPE above */
close(ipipe[1]);
/* Read data output until we run out of stuff to read. */
--
ldv
10 years, 3 months
[linux-pam] #5: multiple pam_namespace unmount problems
by fedora-badges
#5: multiple pam_namespace unmount problems
-----------------------------+------------------------------
Reporter: andersblomdell | Owner: pam-developers@…
Type: defect | Status: new
Priority: major | Component: library
Version: 1.1.x | Keywords:
Blocked By: | Blocking:
-----------------------------+------------------------------
This is essentially a short version of the bug in:
http://bugzilla.redhat.com/show_bug.cgi?id=755216
Essentially pam_namespace (1.1.5) suffers the following problems:
1. The (bind) mounts done in the new namespace is visible in the
original namespace (Error "too many levels of symbolic links").
2. At pam_namespace exit, the original mounting is restored for any
remaining child processes (daemons), which is a security problem.
Patch is attached
--
Ticket URL: <https://fedorahosted.org/linux-pam/ticket/5>
linux-pam <http://fedorahosted.org/linux-pam>
The Linux-PAM (Pluggable Authentication Modules) project
10 years, 3 months
[PATCH] libpam_misc: fix an inconsistency in handling memory allocation errors
by Dmitry V. Levin
When misc_conv fails to allocate memory for pam_response array, it
returns PAM_CONV_ERR. However, when read_string fails to allocate
memory for a response string, it loses the response string and silently
ignores the error, with net result as if EOF has been read.
* libpam_misc/misc_conv.c (read_string): Use strdup instead of x_strdup,
the latter is of no benefit in this case.
Do not ignore potential memory allocation errors returned by strdup,
forward them to misc_conv.
---
libpam_misc/misc_conv.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/libpam_misc/misc_conv.c b/libpam_misc/misc_conv.c
index 3f74eea..be53f34 100644
--- a/libpam_misc/misc_conv.c
+++ b/libpam_misc/misc_conv.c
@@ -210,25 +210,29 @@ static int read_string(int echo, const char *prompt, char **retstr)
}
line[nc] = '\0';
}
- *retstr = x_strdup(line);
+ *retstr = strdup(line);
_pam_overwrite(line);
+ if (!*retstr) {
+ D(("no memory for response string"));
+ nc = -1;
+ }
goto cleanexit; /* return malloc()ed string */
} else if (nc == 0) { /* Ctrl-D */
D(("user did not want to type anything"));
*retstr = NULL;
if (echo) {
fprintf(stderr, "\n");
}
goto cleanexit; /* return malloc()ed "" */
} else if (nc == -1) {
/* Don't loop forever if read() returns -1. */
D(("error reading input from the user: %m"));
if (echo) {
fprintf(stderr, "\n");
}
*retstr = NULL;
goto cleanexit; /* return NULL */
}
--
ldv
10 years, 3 months
[linux-pam] #22: pam_mkhomedir uses user alias username instead of canonical name when creating home directories
by fedora-badges
#22: pam_mkhomedir uses user alias username instead of canonical name when
creating home directories
-------------------------+-------------------------------------------------
Reporter: | Owner: pam-developers@…
musicalvegan0 | Status: new
Type: defect | Component: modules
Priority: major | Keywords: sssd, ipa, active directory,
Version: 1.1.x | mkhomedir
Blocked By: | Blocking:
-------------------------+-------------------------------------------------
When logging in with an "alias" for the first time, mkhomedir will take
the alias of the user instead of looking up the canonical name associated
with the alias. This can lead to the creation of the wrong home directory
and ends up putting the user somewhere other than their home directory.
Please see this listserv archive for additional context:
https://lists.fedorahosted.org/pipermail/sssd-
users/2013-October/001056.html
Steps to reproduce:
1. Create a user account with an alias on a directory server.
2. Configure PAM to authenticate against the directory server.
3. Configure PAM with pam_mkhomedir.so so home directories are created for
first time logins.
4. Login for the first time with the user's alias.
What is expected to happen:
The proper home directory is created and the user is chdir-ed into it.
What actually happens:
This can vary. In my case, the user's home directory path is not retrieved
from the directory server and a fallback home directory is erroneously
created. The user is not started in the proper home directory.
--
Ticket URL: <https://fedorahosted.org/linux-pam/ticket/22>
linux-pam <http://fedorahosted.org/linux-pam>
The Linux-PAM (Pluggable Authentication Modules) project
10 years, 3 months
[PATCH] pam_limits: fix utmp->ut_user handling
by Dmitry V. Levin
ut_user member of struct utmp is a string that is not necessarily
null-terminated, so extra care should be taken when using it.
* modules/pam_limits/pam_limits.c (check_logins): Convert ut->UT_USER to
a null-terminated string and consistently use it where a null-terminated
string is expected.
---
modules/pam_limits/pam_limits.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/modules/pam_limits/pam_limits.c b/modules/pam_limits/pam_limits.c
index e2bc8e1..eabc856 100644
--- a/modules/pam_limits/pam_limits.c
+++ b/modules/pam_limits/pam_limits.c
@@ -270,20 +270,25 @@ check_logins (pam_handle_t *pamh, const char *name, int limit, int ctrl,
continue;
}
if (!pl->flag_numsyslogins) {
+ char user[sizeof(ut->UT_USER) + 1];
+ user[0] = '\0';
+ strncat(user, ut->UT_USER, sizeof(ut->UT_USER));
+
if (((pl->login_limit_def == LIMITS_DEF_USER)
|| (pl->login_limit_def == LIMITS_DEF_GROUP)
|| (pl->login_limit_def == LIMITS_DEF_DEFAULT))
- && strncmp(name, ut->UT_USER, sizeof(ut->UT_USER)) != 0) {
+ && strcmp(name, user) != 0) {
continue;
}
if ((pl->login_limit_def == LIMITS_DEF_ALLGROUP)
- && !pam_modutil_user_in_group_nam_nam(pamh, ut->UT_USER, pl->login_group)) {
+ && !pam_modutil_user_in_group_nam_nam(pamh, user, pl->login_group)) {
continue;
}
if (kill(ut->ut_pid, 0) == -1 && errno == ESRCH) {
/* process does not exist anymore */
pam_syslog(pamh, LOG_WARNING,
- "Stale utmp entry (pid %d) for '%s' ignored", ut->ut_pid, name);
+ "Stale utmp entry (pid %d) for '%s' ignored",
+ ut->ut_pid, user);
continue;
}
}
--
ldv
10 years, 3 months
pam_limits: Verify that the process found from the utmp record is still alive
by Tomas Mraz
The utmp entry for an user might be wrong for example when the process
that should update the entry on logout crashes. This patch by
Christopher Hailey tries to verify that the process that is associated
with the utmp record is still alive. Of course this is not perfectly
race-free but still better than nothing.
OK to commit?
--
Tomas Mraz
No matter how far down the wrong road you've gone, turn back.
Turkish proverb
(You'll never know whether the road is wrong though.)
10 years, 3 months
[PATCH] pam_mkhomedir: check and create home directory for the same user (ticket #22)
by Dmitry V. Levin
Before pam_mkhomedir helper was introduced in commit
7b14630ef39e71f603aeca0c47edf2f384717176, pam_mkhomedir was checking for
existance and creating the same directory - the home directory of the
user NAME returned by pam_get_item(PAM_USER).
The change in behaviour accidentally introduced along with
mkhomedir_helper is not consistent: while the module still checks for
getpwnam(NAME)->pw_dir, the directory created by mkhomedir_helper is
getpwnam(getpwnam(NAME)->pw_name)->pw_dir, which is not necessarily
the same as the directory being checked.
This change brings check and creation back in sync, both handling
getpwnam(NAME)->pw_dir.
* modules/pam_mkhomedir/pam_mkhomedir.c (create_homedir): Replace
"struct passwd *" argument with user's name and home directory.
Pass user's name to MKHOMEDIR_HELPER.
(pam_sm_open_session): Update create_homedir call.
---
modules/pam_mkhomedir/pam_mkhomedir.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/modules/pam_mkhomedir/pam_mkhomedir.c b/modules/pam_mkhomedir/pam_mkhomedir.c
index 5ac8a0f..0b5fc75 100644
--- a/modules/pam_mkhomedir/pam_mkhomedir.c
+++ b/modules/pam_mkhomedir/pam_mkhomedir.c
@@ -103,14 +103,14 @@ _pam_parse (const pam_handle_t *pamh, int flags, int argc, const char **argv,
/* Do the actual work of creating a home dir */
static int
create_homedir (pam_handle_t *pamh, options_t *opt,
- const struct passwd *pwd)
+ const char *user, const char *dir)
{
int retval, child;
struct sigaction newsa, oldsa;
/* Mention what is happening, if the notification fails that is OK */
if (!(opt->ctrl & MKHOMEDIR_QUIET))
- pam_info(pamh, _("Creating directory '%s'."), pwd->pw_dir);
+ pam_info(pamh, _("Creating directory '%s'."), dir);
D(("called."));
@@ -146,7 +146,7 @@ create_homedir (pam_handle_t *pamh, options_t *opt,
/* exec the mkhomedir helper */
args[0] = x_strdup(MKHOMEDIR_HELPER);
- args[1] = pwd->pw_name;
+ args[1] = (char *) user;
args[2] = x_strdup(opt->umask);
args[3] = x_strdup(opt->skeldir);
@@ -181,7 +181,7 @@ create_homedir (pam_handle_t *pamh, options_t *opt,
if (retval != PAM_SUCCESS && !(opt->ctrl & MKHOMEDIR_QUIET)) {
pam_error(pamh, _("Unable to create and initialize directory '%s'."),
- pwd->pw_dir);
+ dir);
}
D(("returning %d", retval));
@@ -230,7 +230,7 @@ pam_sm_open_session (pam_handle_t *pamh, int flags, int argc,
return PAM_SUCCESS;
}
- return create_homedir(pamh, &opt, pwd);
+ return create_homedir(pamh, &opt, user, pwd->pw_dir);
}
/* Ignore */
--
ldv
10 years, 3 months
[linux-pam] #19: logger message uses uninitialized char buffer in group_match() of pam_access.c
by fedora-badges
#19: logger message uses uninitialized char buffer in group_match() of
pam_access.c
-----------------------+------------------------------
Reporter: nickfelt | Owner: pam-developers@…
Type: defect | Status: new
Priority: minor | Component: modules
Version: 1.1.x | Keywords:
Blocked By: | Blocking:
-----------------------+------------------------------
I was debugging my configuration of the pam_access module and noticed some
strange behavior in the debug-level logging messages, which I traced to
the `group_match()` function:
[https://git.fedorahosted.org/cgit/linux-
pam.git/tree/modules/pam_access/pam_access.c#n566]
On line 576 it writes a log message using the `grptok` char buffer, which
has just been declared and not initialized. Presumably the logging call
should either use `tok` instead to print the group name in parentheses, or
the logging call should be moved below the `strncpy()` call, at which
point `grptok` is valid, to print the group name without parentheses. I
would've attached a patch but I wasn't sure which solution was preferable.
At least on my machine, the current code prints an empty string on the
first call to `group_match` and then on each successive call it prints the
value of grptok corresponding to the previous call to `group_match` that
is still in the buffer. I've attached my test-case
`/etc/security/access.conf` and the logging output I saw.
--
Ticket URL: <https://fedorahosted.org/linux-pam/ticket/19>
linux-pam <http://fedorahosted.org/linux-pam>
The Linux-PAM (Pluggable Authentication Modules) project
10 years, 3 months
