[PATCH 0/2] Fix chsh when called as root

[PATCH 0/2] Fix chsh when called as root

[Quentin Rameau]

Hello,

Because chsh was using getpwuid/getpwnam, there was an issue with
subsequent call to readline with tilde expansion which would also call
the same function and overwrite the getpwuid/getpwnam internal storage
struct passwd, replacing passed username results.

[PATCH 1/2] lib/pwdutils: add xgetpwuid

[Quentin Rameau]

---
 include/pwdutils.h |  1 +
 lib/pwdutils.c     | 28 ++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/include/pwdutils.h b/include/pwdutils.h
index a69dd6b45..bea46e57e 100644
--- a/include/pwdutils.h
+++ b/include/pwdutils.h
@@ -5,6 +5,7 @@
 #include <pwd.h>
 
 extern struct passwd *xgetpwnam(const char *username, char **pwdbuf);
+extern struct passwd *xgetpwuid(uid_t uid, char **pwdbuf);
 extern char *xgetlogin(void);
 
 #endif /* UTIL_LINUX_PWDUTILS_H */
diff --git a/lib/pwdutils.c b/lib/pwdutils.c
index 25b4daed0..d54458d65 100644
--- a/lib/pwdutils.c
+++ b/lib/pwdutils.c
@@ -36,6 +36,34 @@ failed:
 	return NULL;
 }
 
+struct passwd *xgetpwuid(uid_t uid, char **pwdbuf)
+{
+	struct passwd *pwd = NULL, *res = NULL;
+	int rc;
+
+	if (!pwdbuf)
+		return NULL;
+
+	*pwdbuf = xmalloc(UL_GETPW_BUFSIZ);
+	pwd = xcalloc(1, sizeof(struct passwd));
+
+	errno = 0;
+	rc = getpwuid_r(uid, pwd, *pwdbuf, UL_GETPW_BUFSIZ, &res);
+	if (rc != 0) {
+		errno = rc;
+		goto failed;
+	}
+	if (!res) {
+		errno = EINVAL;
+		goto failed;
+	}
+	return pwd;
+failed:
+	free(pwd);
+	free(*pwdbuf);
+	return NULL;
+}
+
 char *xgetlogin(void)
 {
 	struct passwd *pw = NULL;
-- 
2.21.0

[PATCH 2/2] chsh: replace getpw unsafe functions with xgetpw

[Quentin Rameau]

---
 login-utils/chsh.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/login-utils/chsh.c b/login-utils/chsh.c
index 9b2761157..a9ebec86f 100644
--- a/login-utils/chsh.c
+++ b/login-utils/chsh.c
@@ -38,6 +38,7 @@
 #include "islocal.h"
 #include "nls.h"
 #include "pathnames.h"
+#include "pwdutils.h"
 #include "setpwnam.h"
 #include "strutils.h"
 #include "xalloc.h"
@@ -253,7 +254,7 @@ static void check_shell(const char *shell)
 
 int main(int argc, char **argv)
 {
-	char *oldshell;
+	char *oldshell, *pwbuf;
 	int nullshell = 0;
 	const uid_t uid = getuid();
 	struct sinfo info = { NULL };
@@ -267,12 +268,12 @@ int main(int argc, char **argv)
 
 	parse_argv(argc, argv, &info);
 	if (!info.username) {
-		pw = getpwuid(uid);
+		pw = xgetpwuid(uid, &pwbuf);
 		if (!pw)
 			errx(EXIT_FAILURE, _("you (user %d) don't exist."),
 			     uid);
 	} else {
-		pw = getpwnam(info.username);
+		pw = xgetpwnam(info.username, &pwbuf);
 		if (!pw)
 			errx(EXIT_FAILURE, _("user \"%s\" does not exist."),
 			     info.username);
-- 
2.21.0

Re: [PATCH 0/2] Fix chsh when called as root

[Karel Zak]

On Sat, Sep 21, 2019 at 08:50:19PM +0200, Quentin Rameau wrote:
> Because chsh was using getpwuid/getpwnam, there was an issue with
> subsequent call to readline with tilde expansion which would also call
> the same function and overwrite the getpwuid/getpwnam internal storage
> struct passwd, replacing passed username results.

Applied, thanks.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 0/2] Fix chsh when called as root

[Quentin Rameau]

> > Because chsh was using getpwuid/getpwnam, there was an issue with
> > subsequent call to readline with tilde expansion which would also call
> > the same function and overwrite the getpwuid/getpwnam internal storage
> > struct passwd, replacing passed username results.
> 
> Applied, thanks.

Thank you!