nss-systemd: pack pw_passwd result into supplied buffer

mcatanzaro · mcatanzaro · commit 92b264676ccd · 2021-09-08T16:19:28.000-05:00
getpwnam_r() guarantees that the strings in the struct passwd that it returns are pointers into the buffer allocated by the application and passed to getpwnam_r(). This means applications may choose to modify the strings in place, as long as the length of the strings is not increased. So it's wrong for us to return a static string here, we really do have to copy it into the application-provided buffer like we do for all the other strings. This is only a theoretical problem since it would be very weird for an application to modify the pw_passwd field, but I spotted this when investigating a similar crash caused by glib editing a different field. See also: https://gitlab.gnome.org/GNOME/glib/-/merge_requests/2244
diff --git a/src/nss-systemd/userdb-glue.c b/src/nss-systemd/userdb-glue.c
@@ -35,6 +35,8 @@ int nss_pack_user_record(
         assert(hr->user_name);
         required = strlen(hr->user_name) + 1;
 
+        required += 2; /* strlen(PASSWORD_SEE_SHADOW) + 1 */
+
         assert_se(rn = user_record_real_name(hr));
         required += strlen(rn) + 1;
 
@@ -51,12 +53,12 @@ int nss_pack_user_record(
                 .pw_name = buffer,
                 .pw_uid = hr->uid,
                 .pw_gid = user_record_gid(hr),
-                .pw_passwd = (char*) PASSWORD_SEE_SHADOW,
         };
 
         assert(buffer);
 
-        pwd->pw_gecos = stpcpy(pwd->pw_name, hr->user_name) + 1;
+        pwd->pw_passwd = stpcpy(pwd->pw_name, hr->user_name) + 1;
+        pwd->pw_gecos = stpcpy(pwd->pw_passwd, PASSWORD_SEE_SHADOW) + 1;
         pwd->pw_dir = stpcpy(pwd->pw_gecos, rn) + 1;
         pwd->pw_shell = stpcpy(pwd->pw_dir, hd) + 1;
         strcpy(pwd->pw_shell, shell);
