newgrp: avoid use of obsolete getpass() function

Read a password from user with termios, and once the password data is no
longer needed ensure it gets overwrote before unallocating memory.

Reference: http://man7.org/linux/man-pages/man3/getpass.3.html
Reference: https://www.securecoding.cert.org/confluence/display/seccode/MSC06-C.+Beware+of+compiler+optimizations
Signed-off-by: Sami Kerola <kerolasa@iki.fi>

diff --git a/login-utils/newgrp.c b/login-utils/newgrp.c
index d492f23ffea690e138cb22794c911b788961dd49..240c359ac18d39b8a3b2c121b562b837edd88d68 100644 (file)
--- a/login-utils/newgrp.c
+++ b/login-utils/newgrp.c
@@ -22,6 +22,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <termios.h>
 #include <unistd.h>
 
 #ifdef HAVE_CRYPT_H
@@ -38,6 +39,49 @@
 #include "pathnames.h"
 #include "xalloc.h"
 
+static char *xgetpass(FILE *input, const char *prompt)
+{
+       char *pass = NULL;
+       struct termios saved, no_echo;
+       const int fd = fileno(input);
+       size_t dummy = 0;
+       ssize_t len;
+
+       fputs(prompt, stdout);
+       if (isatty(fd)) {
+               /* disable echo */
+               tcgetattr(fd, &saved);
+               no_echo = saved;
+               no_echo.c_lflag &= ~ECHO;
+               no_echo.c_lflag |= ECHONL;
+               if (tcsetattr(fd, TCSANOW, &no_echo))
+                       err(EXIT_FAILURE, _("could not set terminal attributes"));
+       }
+       len = getline(&pass, &dummy, input);
+       if (isatty(fd))
+               /* restore terminal */
+               if (tcsetattr(fd, TCSANOW, &saved))
+                       err(EXIT_FAILURE, _("could not set terminal attributes"));
+       if (len < 0)
+               err(EXIT_FAILURE, _("could not getline"));
+       if (0 < len && *(pass + len - 1) == '\n')
+               *(pass + len - 1) = '\0';
+       return pass;
+}
+
+/* Ensure memory is set to value c without compiler optimization getting
+ * into way that could happen with memset(3). */
+static int memset_s(void *v, size_t sz, const int c)
+{
+       volatile unsigned char *p = v;
+
+       if (v == NULL)
+               return EINVAL;
+       while (sz--)
+               *p++ = c;
+       return 0;
+}
+
 /* try to read password from gshadow */
 static char *get_gshadow_pwd(char *groupname)
 {
@@ -110,9 +154,11 @@ static int allow_setgid(struct passwd *pe, struct group *ge)
        if (!(pwd = get_gshadow_pwd(ge->gr_name)))
                pwd = ge->gr_passwd;
 
-       if (pwd && *pwd && (xpwd = getpass(_("Password: ")))) {
+       if (pwd && *pwd && (xpwd = xgetpass(stdin, _("Password: ")))) {
                char *cbuf = crypt(xpwd, pwd);
 
+               memset_s(xpwd, strlen(xpwd), 0);
+               free(xpwd);
                if (!cbuf)
                        warn(_("crypt() failed"));
                else if (strcmp(pwd, cbuf) == 0)