From 8f663a674dc6a48c542299dda1d28d69e0525081 Mon Sep 17 00:00:00 2001 From: Sami Kerola Date: Sun, 14 Sep 2014 18:57:37 +0100 Subject: 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 --- login-utils/newgrp.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) 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 #include #include +#include #include #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) -- cgit v1.2.3-55-g7522