Adjust PAM/NSS snprintf overflow checks for increased robustness (#307)

jonasbardino · web-flow · commit 4cd17ba7a1cd · 2025-08-07T09:00:09.000+02:00
Adjust the `snprintf` overflow checks in PAM and NSS for increased robustness as suggested by @rasmunk . Reworked the get runtime int variable helper to support all possible (long long) integers we may potentially encounter while adding a similar `snprintf` check there. Further analysis of the relevant code confirmed that we already have prior checks in place around all such `snprintf` uses to prevent overflow issues, but it's still better to fix all such potentially unsafe use patterns to prevent them from getting further duplicated without such prior checks.
diff --git a/mig/src/include/auth/migauth.h b/mig/src/include/auth/migauth.h
@@ -1,6 +1,6 @@
 /*
  * migauth.h - PAM and NSS helpers for MiG user authentication
- * Copyright (C) 2003-2024  The MiG Project lead by Brian Vinter
+ * Copyright (C) 2003-2025  The MiG Project lead by the Science HPC Center at UCPH
  *
  * This file is part of MiG
  *
@@ -84,6 +84,14 @@
 /* Default fall-back value used unless given */
 #define PASSWORD_MIN_CLASSES 2
 #endif
+#ifndef INTEGER_MAX_LENGTH
+/* Default fall-back value used unless given */
+/* This size is based on long long integer size i.e. LLONG_MAX in C and
+ * sys.maxsize in python, which should always fit in at most 21 bytes even
+ * when optional sign ('-') and the terminating null-byte is included.
+ */
+#define INTEGER_MAX_LENGTH 21
+#endif
 
 /* Various settings used by ordinary user access */
 #ifndef USER_HOME
@@ -257,13 +265,26 @@ static const int get_runtime_var_int(const char *env_name,
                                      const int define_val)
 {
     /* NOTE: tedious but required juggling between string and integer */
-    char val_str[4];
+
+    /* IMPORTANT: when constructing strings from unknown input we must use
+     * snprintf with the actual buffer size as max size parameter and check
+     * that the returned value was smaller than that size. Any bigger return
+     * value means that the buffer was written but the input truncated when
+     * it attempted to write the returned number of bytes.
+     * https://pubs.opengroup.org/onlinepubs/9799919799/functions/snprintf.html
+     */
+
+    char val_str[INTEGER_MAX_LENGTH];
     /* Convert define_val int to '\0'-terminated string */
-    snprintf(val_str, 3, "%d", define_val);
-    val_str[3] = 0;
+    memset(val_str, 0, INTEGER_MAX_LENGTH);
+    if (INTEGER_MAX_LENGTH <=
+        snprintf(val_str, INTEGER_MAX_LENGTH, "%d", define_val)) {
+        WRITELOGMESSAGE(LOG_ERR, "Overflow in get runtime int var %s: %s\n",
+                        env_name, define_val);
+        return -42;
+    }
     /* Convert lookup back to int */
     return atoi(get_runtime_var(env_name, conf_name, val_str));
-
 }
 
 static const int get_runtime_var_size_t(const char *env_name,
diff --git a/mig/src/libnss-mig/libnss_mig.c b/mig/src/libnss-mig/libnss_mig.c
@@ -2,7 +2,7 @@
  * --- BEGIN_HEADER ---
  *
  * libnss_mig - NSS module for MiG user authentication
- * Copyright (C) 2003-2022  The MiG Project lead by Brian Vinter
+ * Copyright (C) 2003-2025  The MiG Project lead by the Science HPC Center at UCPH
  *
  * This file is part of MiG
  *
@@ -174,6 +174,14 @@ _nss_mig_getpwnam_r(const char *name,
      */
     int keep_trying = 1;
 
+    /* IMPORTANT: when constructing strings from unknown input we must use
+     * snprintf with the actual buffer size as max size parameter and check
+     * that the returned value was smaller than that size. Any bigger return
+     * value means that the buffer was written but the input truncated when
+     * it attempted to write the returned number of bytes.
+     * https://pubs.opengroup.org/onlinepubs/9799919799/functions/snprintf.html
+     */
+
 #ifdef ENABLE_SHARELINK
     /* Optional anonymous share link access:
        - username must have fixed length matching get_sharelink_length()
@@ -183,7 +191,7 @@ _nss_mig_getpwnam_r(const char *name,
     if (keep_trying && name_len == get_sharelink_length()) {
         WRITELOGMESSAGE(LOG_DEBUG, "Checking for sharelink: %s\n", name);
         memset(pathbuf, 0, PATH_BUF_LEN);
-        if (PATH_BUF_LEN ==
+        if (PATH_BUF_LEN <=
             snprintf(pathbuf, PATH_BUF_LEN, "%s/%s/%s",
                      get_sharelink_home(), SHARELINK_SUBDIR, name)) {
             WRITELOGMESSAGE(LOG_WARNING,
@@ -237,7 +245,7 @@ _nss_mig_getpwnam_r(const char *name,
     if (keep_trying && name_len == get_jobsidmount_length()) {
         WRITELOGMESSAGE(LOG_DEBUG, "Checking for jobsidmount: %s\n", name);
         memset(pathbuf, 0, PATH_BUF_LEN);
-        if (PATH_BUF_LEN ==
+        if (PATH_BUF_LEN <=
             snprintf(pathbuf, PATH_BUF_LEN, "%s/%s",
                      get_jobsidmount_home(), name)) {
             WRITELOGMESSAGE(LOG_WARNING,
@@ -290,7 +298,7 @@ _nss_mig_getpwnam_r(const char *name,
     if (keep_trying && name_len == get_jupytersidmount_length()) {
         WRITELOGMESSAGE(LOG_DEBUG, "Checking for jupytersidmount: %s\n", name);
         memset(pathbuf, 0, PATH_BUF_LEN);
-        if (PATH_BUF_LEN ==
+        if (PATH_BUF_LEN <=
             snprintf(pathbuf, PATH_BUF_LEN, "%s/%s",
                      get_jupytersidmount_home(), name)) {
             WRITELOGMESSAGE(LOG_WARNING,
diff --git a/mig/src/libpam-mig/libpam_mig.c b/mig/src/libpam-mig/libpam_mig.c
@@ -526,6 +526,14 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t * pamh, int flags,
 
     /* Basic validation of username before use anywhere in paths or python */
 
+    /* IMPORTANT: when constructing strings from unknown input we must use
+     * snprintf with the actual buffer size as max size parameter and check
+     * that the returned value was smaller than that size. Any bigger return
+     * value means that the buffer was written but the input truncated when
+     * it attempted to write the returned number of bytes.
+     * https://pubs.opengroup.org/onlinepubs/9799919799/functions/snprintf.html
+     */
+
     /* Since we rely on mapping the username to a path on disk,
        double check that the name does not contain path traversal attempts
        after basic input validation for only safe characters. */
@@ -539,7 +547,7 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t * pamh, int flags,
     } else {
         valid_username = true;
         /* pUsername is validated enough to be safely used in python calls */
-        if (USERNAME_MAX_LENGTH ==
+        if (USERNAME_MAX_LENGTH <=
             snprintf(safeUsername, USERNAME_MAX_LENGTH, "%s", pUsername)) {
             WRITELOGMESSAGE(LOG_WARNING,
                             "Safe username construction failed for: %s\n",
@@ -714,7 +722,7 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t * pamh, int flags,
     WRITELOGMESSAGE(LOG_DEBUG, "Checking for sharelink: %s\n", pUsername);
     if (strlen(pUsername) == get_sharelink_length()) {
         char share_path[MAX_PATH_LENGTH];
-        if (MAX_PATH_LENGTH ==
+        if (MAX_PATH_LENGTH <=
             snprintf(share_path, MAX_PATH_LENGTH, "%s/%s/%s",
                      get_sharelink_home(), SHARELINK_SUBDIR, pUsername)) {
             WRITELOGMESSAGE(LOG_WARNING,
@@ -780,7 +788,7 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t * pamh, int flags,
     WRITELOGMESSAGE(LOG_DEBUG, "Checking for jobsidmount: %s\n", pUsername);
     if (strlen(pUsername) == get_jobsidmount_length()) {
         char share_path[MAX_PATH_LENGTH];
-        if (MAX_PATH_LENGTH ==
+        if (MAX_PATH_LENGTH <=
             snprintf(share_path, MAX_PATH_LENGTH, "%s/%s",
                      get_jobsidmount_home(), pUsername)) {
             WRITELOGMESSAGE(LOG_WARNING,
@@ -860,7 +868,7 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t * pamh, int flags,
     WRITELOGMESSAGE(LOG_DEBUG, "Checking for jupytersidmount: %s\n", pUsername);
     if (strlen(pUsername) == get_jupytersidmount_length()) {
         char share_path[MAX_PATH_LENGTH];
-        if (MAX_PATH_LENGTH ==
+        if (MAX_PATH_LENGTH <=
             snprintf(share_path, MAX_PATH_LENGTH, "%s/%s",
                      get_jupytersidmount_home(), pUsername)) {
             WRITELOGMESSAGE(LOG_WARNING,
@@ -952,7 +960,7 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t * pamh, int flags,
     }
 
     char auth_filename[MAX_PATH_LENGTH];
-    if (MAX_PATH_LENGTH ==
+    if (MAX_PATH_LENGTH <=
         snprintf(auth_filename, MAX_PATH_LENGTH, "%s/.%s/%s", pw->pw_dir,
                  get_service_dir(pService), PASSWORD_FILENAME)) {
         WRITELOGMESSAGE(LOG_WARNING,
