rpms/pam
7
0
Fork 0
Code Issues Pull Requests Releases Activity

Fix static analyzer detected issues

Browse Source 
pam_env: fixes for NULL environment variables

Resolves: RHEL-36475

Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
This commit is contained in:
Iker Pedrosa 2024-06-18 12:56:40 +02:00
parent 8c4a8fc39d
commit 8cc986a906
3 changed files with 309 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

86
pam-1.6.1-pam-env-econf-read-file-fixes.patch Normal file
View File

@ -0,0 +1,86 @@
From aabd5314a6d76968c377969b49118a2df3f97003 Mon Sep 17 00:00:00 2001
From: "Dmitry V. Levin" <ldv@strace.io>
Date: Sun, 19 May 2024 15:00:00 +0000
Subject: [PATCH 1/2] pam_env: fix NULL dereference on error path in
econf_read_file
* modules/pam_env/pam_env.c [USE_ECONF] (econf_read_file): Handle NULL
value returned by econf_getStringValue().
Resolves: https://github.com/linux-pam/linux-pam/issues/796
---
modules/pam_env/pam_env.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/modules/pam_env/pam_env.c b/modules/pam_env/pam_env.c
index 2cc58228..6d39bb24 100644
--- a/modules/pam_env/pam_env.c
+++ b/modules/pam_env/pam_env.c
@@ -287,7 +287,7 @@ econf_read_file(const pam_handle_t *pamh, const char *filename, const char *deli
char *val;
error = econf_getStringValue (key_file, NULL, keys[i], &val);
- if (error != ECONF_SUCCESS) {
+ if (error != ECONF_SUCCESS || val == NULL) {
pam_syslog(pamh, LOG_ERR, "Unable to get string from key %s: %s",
keys[i],
econf_errString(error));
--
2.45.1
From 75292685a625153c6e28bdd820e97421c258c04a Mon Sep 17 00:00:00 2001
From: "Dmitry V. Levin" <ldv@strace.io>
Date: Sun, 19 May 2024 15:00:00 +0000
Subject: [PATCH 2/2] pam_env: fix error handling in econf_read_file
* modules/pam_env/pam_env.c [USE_ECONF] (econf_read_file): Make sure
the returned array of strings is properly initialized
when econf_getStringValue() fails to return a value.
---
modules/pam_env/pam_env.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/modules/pam_env/pam_env.c b/modules/pam_env/pam_env.c
index 6d39bb24..7c146439 100644
--- a/modules/pam_env/pam_env.c
+++ b/modules/pam_env/pam_env.c
@@ -273,7 +273,7 @@ econf_read_file(const pam_handle_t *pamh, const char *filename, const char *deli
return PAM_ABORT;
}
- *lines = malloc((key_number +1)* sizeof(char**));
+ *lines = calloc((key_number + 1), sizeof(char**));
if (*lines == NULL) {
pam_syslog(pamh, LOG_ERR, "Cannot allocate memory.");
econf_free(keys);
@@ -281,8 +281,7 @@ econf_read_file(const pam_handle_t *pamh, const char *filename, const char *deli
return PAM_BUF_ERR;
}
- (*lines)[key_number] = 0;
-
+ size_t n = 0;
for (size_t i = 0; i < key_number; i++) {
char *val;
@@ -293,7 +292,7 @@ econf_read_file(const pam_handle_t *pamh, const char *filename, const char *deli
econf_errString(error));
} else {
econf_unescnl(val);
- if (asprintf(&(*lines)[i],"%s%c%s", keys[i], delim[0], val) < 0) {
+ if (asprintf(&(*lines)[n],"%s%c%s", keys[i], delim[0], val) < 0) {
pam_syslog(pamh, LOG_ERR, "Cannot allocate memory.");
econf_free(keys);
econf_freeFile(key_file);
@@ -303,6 +302,7 @@ econf_read_file(const pam_handle_t *pamh, const char *filename, const char *deli
return PAM_BUF_ERR;
}
free (val);
+ n++;
}
}
--
2.45.1

212
pam-1.6.1-sast-fixes.patch Normal file
View File

@ -0,0 +1,212 @@
From 5eccaf9b3488d3f6da800281363697e4e4834e77 Mon Sep 17 00:00:00 2001
From: Iker Pedrosa <ipedrosa@redhat.com>
Date: Wed, 22 May 2024 11:16:28 +0200
Subject: [PATCH 1/5] pam_faillock: close the audit socket after use
* modules/pam_faillock/pam_faillock.c (check_tally): Close the audit
socket when it will no longer be used.
```
Error: RESOURCE_LEAK (CWE-772):
Linux-PAM-1.6.0/modules/pam_faillock/pam_faillock.c:247: open_fn: Returning handle opened by "audit_open".
Linux-PAM-1.6.0/modules/pam_faillock/pam_faillock.c:247: var_assign: Assigning: "audit_fd" = handle returned from "audit_open()".
Linux-PAM-1.6.0/modules/pam_faillock/pam_faillock.c:256: noescape: Resource "audit_fd" is not freed or pointed-to in "audit_log_user_message".
Linux-PAM-1.6.0/modules/pam_faillock/pam_faillock.c:258: leaked_handle: Handle variable "audit_fd" going out of scope leaks the handle.
256| audit_log_user_message(audit_fd, AUDIT_RESP_ACCT_UNLOCK_TIMED, buf,
257| rhost, NULL, tty, 1);
258|-> }
259| #endif
260| opts->flags |= FAILLOCK_FLAG_UNLOCKED;
```
Resolves: https://issues.redhat.com/browse/RHEL-36475
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
---
modules/pam_faillock/pam_faillock.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/modules/pam_faillock/pam_faillock.c b/modules/pam_faillock/pam_faillock.c
index e636a24e..f39a9d95 100644
--- a/modules/pam_faillock/pam_faillock.c
+++ b/modules/pam_faillock/pam_faillock.c
@@ -255,6 +255,7 @@ check_tally(pam_handle_t *pamh, struct options *opts, struct tally_data *tallies
snprintf(buf, sizeof(buf), "op=pam_faillock suid=%u ", opts->uid);
audit_log_user_message(audit_fd, AUDIT_RESP_ACCT_UNLOCK_TIMED, buf,
rhost, NULL, tty, 1);
+ audit_close(audit_fd);
}
#endif
opts->flags |= FAILLOCK_FLAG_UNLOCKED;
--
2.45.2
From d00f6cb366b492de455f9b72fcbd2e49abf323e0 Mon Sep 17 00:00:00 2001
From: Iker Pedrosa <ipedrosa@redhat.com>
Date: Wed, 22 May 2024 11:20:02 +0200
Subject: [PATCH 2/5] pam_rootok: close the audit socket on error path
* modules/pam_rootok/pam_rootok.c (log_callback): Close the audit socket
if vasprintf returned an error.
```
Error: RESOURCE_LEAK (CWE-772):
Linux-PAM-1.6.0/modules/pam_rootok/pam_rootok.c:59: open_fn: Returning handle opened by "audit_open".
Linux-PAM-1.6.0/modules/pam_rootok/pam_rootok.c:59: var_assign: Assigning: "audit_fd" = handle returned from "audit_open()".
Linux-PAM-1.6.0/modules/pam_rootok/pam_rootok.c:69: leaked_handle: Handle variable "audit_fd" going out of scope leaks the handle.
67| va_end(ap);
68| if (ret < 0) {
69|-> return 0;
70| }
71| audit_log_user_avc_message(audit_fd, AUDIT_USER_AVC, buf, NULL, NULL,
```
Resolves: https://issues.redhat.com/browse/RHEL-36475
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
---
modules/pam_rootok/pam_rootok.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/modules/pam_rootok/pam_rootok.c b/modules/pam_rootok/pam_rootok.c
index 6d2dfa07..1b88fb19 100644
--- a/modules/pam_rootok/pam_rootok.c
+++ b/modules/pam_rootok/pam_rootok.c
@@ -66,6 +66,7 @@ log_callback (int type UNUSED, const char *fmt, ...)
ret = vasprintf (&buf, fmt, ap);
va_end(ap);
if (ret < 0) {
+ audit_close(audit_fd);
return 0;
}
audit_log_user_avc_message(audit_fd, AUDIT_USER_AVC, buf, NULL, NULL,
--
2.45.2
From 1ca5bfed50bd9f6c2f1e3e36c2df3253923dadf6 Mon Sep 17 00:00:00 2001
From: Iker Pedrosa <ipedrosa@redhat.com>
Date: Wed, 22 May 2024 12:27:00 +0200
Subject: [PATCH 3/5] pam_timestamp: close the timestamp file on error path
* modules/pam_timestamp/pam_timestamp.c (pam_sm_authenticate)
[WITH_OPENSSL]: Close the timestamp file if hmac_size returned
an error.
```
Error: RESOURCE_LEAK (CWE-772):
Linux-PAM-1.6.0/modules/pam_timestamp/pam_timestamp.c:450: open_fn: Returning handle opened by "open". [Note: The source code implementation of the function has been overridden by a user model.]
Linux-PAM-1.6.0/modules/pam_timestamp/pam_timestamp.c:450: var_assign: Assigning: "fd" = handle returned from "open(path, 131072)".
Linux-PAM-1.6.0/modules/pam_timestamp/pam_timestamp.c:460: noescape: Resource "fd" is not freed or pointed-to in "fstat".
Linux-PAM-1.6.0/modules/pam_timestamp/pam_timestamp.c:484: leaked_handle: Handle variable "fd" going out of scope leaks the handle.
482| #ifdef WITH_OPENSSL
483| if (hmac_size(pamh, debug, &maclen)) {
484|-> return PAM_AUTH_ERR;
485| }
486| #else
```
Resolves: https://issues.redhat.com/browse/RHEL-36475
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
---
modules/pam_timestamp/pam_timestamp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/modules/pam_timestamp/pam_timestamp.c b/modules/pam_timestamp/pam_timestamp.c
index 7c5457c4..edecc052 100644
--- a/modules/pam_timestamp/pam_timestamp.c
+++ b/modules/pam_timestamp/pam_timestamp.c
@@ -481,6 +481,7 @@ pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc, const char **argv)
#ifdef WITH_OPENSSL
if (hmac_size(pamh, debug, &maclen)) {
+ close(fd);
return PAM_AUTH_ERR;
}
#else
--
2.45.2
From 667204d7e3e4a0341c529f7566d62dd64dd80866 Mon Sep 17 00:00:00 2001
From: Iker Pedrosa <ipedrosa@redhat.com>
Date: Wed, 22 May 2024 12:25:34 +0200
Subject: [PATCH 4/5] pam_namespace: free SELinux context
* modules/pam_namespace/pam_namespace.c [WITH_SELINUX] (form_context):
Free SELinux context before returning.
```
Error: RESOURCE_LEAK (CWE-772):
Linux-PAM-1.6.0/modules/pam_namespace/pam_namespace.c:928: alloc_arg: "getexeccon" allocates memory that is stored into "scon".
Linux-PAM-1.6.0/modules/pam_namespace/pam_namespace.c:1004: leaked_storage: Variable "scon" going out of scope leaks the storage it points to.
1002| }
1003| /* Should never get here */
1004|-> return PAM_SUCCESS;
1005| }
1006| #endif
```
Resolves: https://issues.redhat.com/browse/RHEL-36475
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
---
modules/pam_namespace/pam_namespace.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/modules/pam_namespace/pam_namespace.c b/modules/pam_namespace/pam_namespace.c
index e499d95a..781dac20 100644
--- a/modules/pam_namespace/pam_namespace.c
+++ b/modules/pam_namespace/pam_namespace.c
@@ -1003,6 +1003,7 @@ static int form_context(const struct polydir_s *polyptr,
return rc;
}
/* Should never get here */
+ freecon(scon);
return PAM_SUCCESS;
}
#endif
--
2.45.2
From bd2f695b3d89efe0c52bba975f9540634125178a Mon Sep 17 00:00:00 2001
From: Iker Pedrosa <ipedrosa@redhat.com>
Date: Wed, 22 May 2024 12:29:07 +0200
Subject: [PATCH 5/5] pam_namespace: free SELinux context on error path
* modules/pam_namespace/pam_namespace.c (create_polydir) [WITH_SELINUX]:
Free SELinux context in case of an error.
```
Error: RESOURCE_LEAK (CWE-772):
Linux-PAM-1.6.0/modules/pam_namespace/pam_namespace.c:1433: alloc_arg: "getfscreatecon_raw" allocates memory that is stored into "oldcon_raw".
Linux-PAM-1.6.0/modules/pam_namespace/pam_namespace.c:1462: leaked_storage: Variable "oldcon_raw" going out of scope leaks the storage it points to.
1460| pam_syslog(idata->pamh, LOG_ERR,
1461| "Error creating directory %s: %m", dir);
1462|-> return PAM_SESSION_ERR;
1463| }
1464|
```
Resolves: https://issues.redhat.com/browse/RHEL-36475
Signed-off-by: Iker Pedrosa <ipedrosa@redhat.com>
---
modules/pam_namespace/pam_namespace.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/modules/pam_namespace/pam_namespace.c b/modules/pam_namespace/pam_namespace.c
index 781dac20..2dab49ef 100644
--- a/modules/pam_namespace/pam_namespace.c
+++ b/modules/pam_namespace/pam_namespace.c
@@ -1462,6 +1462,9 @@ static int create_polydir(struct polydir_s *polyptr,
if (rc == -1) {
pam_syslog(idata->pamh, LOG_ERR,
"Error creating directory %s: %m", dir);
+#ifdef WITH_SELINUX
+ freecon(oldcon_raw);
+#endif
return PAM_SESSION_ERR;
}
--
2.45.2

12
pam.spec
View File

@ -4,7 +4,7 @@
Summary: An extensible library which provides authentication for applications Summary: An extensible library which provides authentication for applications
Name: pam Name: pam
Version: 1.6.1 Version: 1.6.1
Release: 1%{?dist} Release: 2%{?dist}
# The library is BSD licensed with option to relicense as GPLv2+ # The library is BSD licensed with option to relicense as GPLv2+
# - this option is redundant as the BSD license allows that anyway. # - this option is redundant as the BSD license allows that anyway.
# pam_timestamp and pam_loginuid modules are GPLv2+. # pam_timestamp and pam_loginuid modules are GPLv2+.
@ -25,6 +25,10 @@ Source18: https://www.gnu.org/licenses/old-licenses/gpl-2.0.txt
Patch1: pam-1.6.0-redhat-modules.patch Patch1: pam-1.6.0-redhat-modules.patch
Patch2: pam-1.6.1-noflex.patch Patch2: pam-1.6.1-noflex.patch
Patch3: pam-1.5.3-unix-nomsg.patch Patch3: pam-1.5.3-unix-nomsg.patch
# https://github.com/linux-pam/linux-pam/commit/bd2f695b3d89efe0c52bba975f9540634125178a
Patch4: pam-1.6.1-sast-fixes.patch
# https://github.com/linux-pam/linux-pam/commit/aabd5314a6d76968c377969b49118a2df3f97003
Patch5: pam-1.6.1-pam-env-econf-read-file-fixes.patch
%{load:%{SOURCE3}} %{load:%{SOURCE3}}
@ -118,6 +122,8 @@ cp %{SOURCE18} .
%patch -P 1 -p1 -b .redhat-modules %patch -P 1 -p1 -b .redhat-modules
%patch -P 2 -p1 -b .noflex %patch -P 2 -p1 -b .noflex
%patch -P 3 -p1 -b .nomsg %patch -P 3 -p1 -b .nomsg
%patch -P 4 -p1 -b .sast-fixes
%patch -P 5 -p1 -b .pam-env-econf-read-file-fixes
autoreconf -i autoreconf -i
@ -356,6 +362,10 @@ done
%{_pam_libdir}/libpam_misc.so.%{so_ver}* %{_pam_libdir}/libpam_misc.so.%{so_ver}*
%changelog %changelog
* Tue Jun 18 2024 Iker Pedrosa <ipedrosa@redhat.com> - 1.6.1-2
- Fix static analyzer detected issues. Resolves: RHEL-36475
- pam_env: fixes for NULL environment variables
* Wed Apr 10 2024 Iker Pedrosa <ipedrosa@redhat.com> - 1.6.1-1 * Wed Apr 10 2024 Iker Pedrosa <ipedrosa@redhat.com> - 1.6.1-1
- Rebase to release 1.6.1 - Rebase to release 1.6.1
- Disable nis support - Disable nis support