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>

1 files changed, 47 insertions, 1 deletions

diff --git a/login-utils/newgrp.c b/login-utils/newgrp.c
index d492f23ff..240c359ac 100644
--- 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)