From d57ab22133654033ee1da89f128a81572d320985 Mon Sep 17 00:00:00 2001

From: Tomas Mraz <tmraz@fedoraproject.org>

Date: Thu, 20 Dec 2018 13:59:25 +0100

Subject: [PATCH] pam_motd: Cleanup the code and avoid unnecessary logging

The pam_motd module will not log if the default motd.d directories

are missing.

Also cleanup some code cleanliness issues and fix compilation

warnings.

* modules/pam_motd/pam_motd.c: Constification of constant strings.

  (try_to_display_directory): Removed unused function.

  (pam_split_string): Replace uint with unsigned int. Fix warnings.

  (compare_strings): Fix warnings by proper constification.

  (try_to_display_directories_with_overrides): Cleanups. Switch

  off the logging if the motd.d directories are missing and they

  are default ones.

  (pam_sm_open_session): Cleanup warnings. Pass the information

  to try_to_display_directories_with_overrides() that non-default

  motd options are used.

---

 modules/pam_motd/pam_motd.c | 88 ++++++++++++++++---------------------

 1 file changed, 37 insertions(+), 51 deletions(-)

diff --git a/modules/pam_motd/pam_motd.c b/modules/pam_motd/pam_motd.c

index ec3ebd58..dbd718b6 100644

--- a/modules/pam_motd/pam_motd.c

+++ b/modules/pam_motd/pam_motd.c

@@ -22,6 +22,7 @@

 #include <sys/stat.h>

 #include <pwd.h>

 #include <syslog.h>

+#include <errno.h>

 #include <security/_pam_macros.h>

 #include <security/pam_ext.h>

@@ -48,8 +49,8 @@ pam_sm_close_session (pam_handle_t *pamh UNUSED, int flags UNUSED,

      return PAM_IGNORE;

 }

-static char default_motd[] = DEFAULT_MOTD;

-static char default_motd_dir[] = DEFAULT_MOTD_D;

+static const char default_motd[] = DEFAULT_MOTD;

+static const char default_motd_dir[] = DEFAULT_MOTD_D;

 static void try_to_display_fd(pam_handle_t *pamh, int fd)

 {

@@ -75,28 +76,6 @@ static void try_to_display_fd(pam_handle_t *pamh, int fd)

     _pam_drop(mtmp);

 }

-static void try_to_display_directory(pam_handle_t *pamh, const char *dirname)

-{

-    DIR *dirp;

-

-    dirp = opendir(dirname);

-

-    if (dirp != NULL) {

-	struct dirent *entry;

-

-	while ((entry = readdir(dirp))) {

-	    int fd = openat(dirfd(dirp), entry->d_name, O_RDONLY);

-

-	    if (fd >= 0) {

-		try_to_display_fd(pamh, fd);

-		close(fd);

-	    }

-	}

-

-	closedir(dirp);

-    }

-}

-

 /*

  * Split a DELIM-separated string ARG into an array.

  * Outputs a newly allocated array of strings OUT_ARG_SPLIT

@@ -104,14 +83,14 @@ static void try_to_display_directory(pam_handle_t *pamh, const char *dirname)

  * Returns 0 in case of error, 1 in case of success.

  */

 static int pam_split_string(const pam_handle_t *pamh, char *arg, char delim,

-			    char ***out_arg_split, uint *out_num_strs)

+			    char ***out_arg_split, unsigned int *out_num_strs)

 {

     char *arg_extracted = NULL;

     const char *arg_ptr = arg;

     char **arg_split = NULL;

     char delim_str[2];

-    int i = 0;

-    uint num_strs = 0;

+    unsigned int i = 0;

+    unsigned int num_strs = 0;

     int retval = 0;

     delim_str[0] = delim;

@@ -126,7 +105,7 @@ static int pam_split_string(const pam_handle_t *pamh, char *arg, char delim,

 	arg_ptr = strchr(arg_ptr + sizeof(const char), delim);

     }

-    arg_split = (char **)calloc(num_strs, sizeof(char *));

+    arg_split = calloc(num_strs, sizeof(char *));

     if (arg_split == NULL) {

 	pam_syslog(pamh, LOG_CRIT, "pam_motd: failed to allocate string array");

 	goto out;

@@ -180,10 +159,10 @@ static int join_dir_strings(char **strp_out, const char *a_str, const char *b_st

     return retval;

 }

-static int compare_strings(const void * a, const void * b)

+static int compare_strings(const void *a, const void *b)

 {

-    const char *a_str = *(char **)a;

-    const char *b_str = *(char **)b;

+    const char *a_str = *(const char * const *)a;

+    const char *b_str = *(const char * const *)b;

     if (a_str == NULL && b_str == NULL) {

         return 0;

@@ -205,13 +184,13 @@ static int filter_dirents(const struct dirent *d)

 }

 static void try_to_display_directories_with_overrides(pam_handle_t *pamh,

-	char **motd_dir_path_split, int num_motd_dirs)

+	char **motd_dir_path_split, unsigned int num_motd_dirs, int report_missing)

 {

     struct dirent ***dirscans = NULL;

-    int *dirscans_sizes = NULL;

-    int dirscans_size_total = 0;

+    unsigned int *dirscans_sizes = NULL;

+    unsigned int dirscans_size_total = 0;

     char **dirnames_all = NULL;

-    int i;

+    unsigned int i;

     int i_dirnames = 0;

     if (pamh == NULL || motd_dir_path_split == NULL) {

@@ -221,29 +200,31 @@ static void try_to_display_directories_with_overrides(pam_handle_t *pamh,

 	goto out;

     }

-    if ((dirscans = (struct dirent ***)calloc(num_motd_dirs,

-	    sizeof(struct dirent **))) == NULL) {

+    if ((dirscans = calloc(num_motd_dirs, sizeof(struct dirent **))) == NULL) {

 	pam_syslog(pamh, LOG_CRIT, "pam_motd: failed to allocate dirent arrays");

 	goto out;

     }

-    if ((dirscans_sizes = (int *)calloc(num_motd_dirs, sizeof(int))) == NULL) {

+    if ((dirscans_sizes = calloc(num_motd_dirs, sizeof(int))) == NULL) {

 	pam_syslog(pamh, LOG_CRIT, "pam_motd: failed to allocate dirent array sizes");

 	goto out;

     }

     for (i = 0; i < num_motd_dirs; i++) {

-	dirscans_sizes[i] = scandir(motd_dir_path_split[i], &(dirscans[i]),

+	int rv;

+	rv = scandir(motd_dir_path_split[i], &(dirscans[i]),

 		filter_dirents, alphasort);

-	if (dirscans_sizes[i] < 0) {

-	    pam_syslog(pamh, LOG_ERR, "pam_motd: error scanning directory %s", motd_dir_path_split[i]);

-	    dirscans_sizes[i] = 0;

+	if (rv < 0) {

+	    if (errno != ENOENT || report_missing) {

+		pam_syslog(pamh, LOG_ERR, "pam_motd: error scanning directory %s: %m",

+		    motd_dir_path_split[i]);

+	    }

+	    dirscans_sizes[i] = rv;

 	}

 	dirscans_size_total += dirscans_sizes[i];

     }

     /* Allocate space for all file names found in the directories, including duplicates. */

-    if ((dirnames_all = (char **)calloc(dirscans_size_total,

-	    sizeof(char *))) == NULL) {

+    if ((dirnames_all = calloc(dirscans_size_total, sizeof(char *))) == NULL) {

 	pam_syslog(pamh, LOG_CRIT, "pam_motd: failed to allocate dirname array");

 	goto out;

     }

@@ -253,7 +234,7 @@ static void try_to_display_directories_with_overrides(pam_handle_t *pamh,

     }

     for (i = 0; i < num_motd_dirs; i++) {

-	int j;

+	unsigned int j;

 	for (j = 0; j < dirscans_sizes[i]; j++) {

 	    dirnames_all[i_dirnames] = dirscans[i][j]->d_name;

@@ -265,7 +246,7 @@ static void try_to_display_directories_with_overrides(pam_handle_t *pamh,

 	    sizeof(const char *), compare_strings);

     for (i = 0; i < dirscans_size_total; i++) {

-	int j;

+	unsigned int j;

 	if (dirnames_all[i] == NULL) {

 	    continue;

@@ -301,7 +282,8 @@ static void try_to_display_directories_with_overrides(pam_handle_t *pamh,

   out:

     _pam_drop(dirnames_all);

     for (i = 0; i < num_motd_dirs; i++) {

-	int j;

+	unsigned int j;

+

 	for (j = 0; j < dirscans_sizes[i]; j++) {

 	    _pam_drop(dirscans[i][j]);

 	}

@@ -319,12 +301,13 @@ int pam_sm_open_session(pam_handle_t *pamh, int flags,

     int retval = PAM_IGNORE;

     const char *motd_path = NULL;

     char *motd_path_copy = NULL;

-    int num_motd_paths = 0;

+    unsigned int num_motd_paths = 0;

     char **motd_path_split = NULL;

     const char *motd_dir_path = NULL;

     char *motd_dir_path_copy = NULL;

-    int num_motd_dir_paths = 0;

+    unsigned int num_motd_dir_paths = 0;

     char **motd_dir_path_split = NULL;

+    int report_missing;

     if (flags & PAM_SILENT) {

 	return retval;

@@ -360,6 +343,9 @@ int pam_sm_open_session(pam_handle_t *pamh, int flags,

     if (motd_path == NULL && motd_dir_path == NULL) {

 	motd_path = default_motd;

 	motd_dir_path = default_motd_dir;

+	report_missing = 0;

+    } else {

+	report_missing = 1;

     }

     if (motd_path != NULL) {

@@ -385,7 +371,7 @@ int pam_sm_open_session(pam_handle_t *pamh, int flags,

     }

     if (motd_path_split != NULL) {

-	int i;

+	unsigned int i;

 	for (i = 0; i < num_motd_paths; i++) {

 	    int fd = open(motd_path_split[i], O_RDONLY, 0);

@@ -402,7 +388,7 @@ int pam_sm_open_session(pam_handle_t *pamh, int flags,

     if (motd_dir_path_split != NULL)

 	try_to_display_directories_with_overrides(pamh, motd_dir_path_split,

-		num_motd_dir_paths);

+		num_motd_dir_paths, report_missing);

   out:

     _pam_drop(motd_path_copy);

-- 

2.37.3

From c2c0434bd634a817f2b16ce7f58fc96c04e88b03 Mon Sep 17 00:00:00 2001

From: "Dmitry V. Levin" <ldv@altlinux.org>

Date: Sun, 26 Apr 2020 11:12:59 +0000

Subject: [PATCH] pam_motd: fix NULL dereference when at least one of motd

 directories is not available

* modules/pam_motd/pam_motd.c

(try_to_display_directories_with_overrides): Do not assign -1U to

dirscans_sizes[i] when scandir(motd_dir_path_split[i]) returns an error.

Resolves: https://bugzilla.altlinux.org/38389

Fixes: d57ab221 ("pam_motd: Cleanup the code and avoid unnecessary logging")

---

 modules/pam_motd/pam_motd.c | 1 +

 1 file changed, 1 insertion(+)

diff --git a/modules/pam_motd/pam_motd.c b/modules/pam_motd/pam_motd.c

index df09b7d0..8147c6fd 100644

--- a/modules/pam_motd/pam_motd.c

+++ b/modules/pam_motd/pam_motd.c

@@ -219,6 +219,7 @@ static void try_to_display_directories_with_overrides(pam_handle_t *pamh,

 		pam_syslog(pamh, LOG_ERR, "pam_motd: error scanning directory %s: %m",

 		    motd_dir_path_split[i]);

 	    }

+	} else {

 	    dirscans_sizes[i] = rv;

 	}

 	dirscans_size_total += dirscans_sizes[i];

-- 

2.37.3