On Tue, Apr 10, 2018 at 06:49:10PM +0200, Han Boetes wrote: > I got a problem report from Mark Willson: > > "I recently installed mg (via the Debian package) under WSL on Windows > 10. > I found that the 'backup-to-home-directory' option didn't work. > > The cause of this appears to be that getlogin under WSL returns NULL, > probably due to my use of wsltty to invoke bash. The patch below fixes > the issue for me:" > > [snip] > - if ((un = getlogin()) != NULL) > + if ((un = getenv("LOGNAME")) != NULL) > [snip] > > Which put me onto the track of what was going on. I found the > following in the Linux manpage: > > BUGS > Unfortunately, it is often rather easy to fool getlogin(). > Sometimes > it does not work at all, because some program messed up the utmp > file. > Often, it gives only the first 8 characters of the login name. > The > user currently logged in on the controlling terminal of our > program > need not be the user who started it. Avoid getlogin() for > security- > related purposes. > > Note that glibc does not follow the POSIX specification and uses > stdin > instead of /dev/tty. A bug. (Other recent systems, like SunOS 5.8 > and > HP-UX 11.11 and FreeBSD 4.8 all return the login name also when > stdin > is redirected.) > > Nobody knows precisely what cuserid() does; avoid it in portable > pro‐ > grams. Or avoid it altogether: use getpwuid(geteuid()) instead, > if > that is what you meant. Do not use cuserid(). > > So I started looking at the code and rewrote it a bit, which I think > makes it more portable and removes a syscall in the process. I do > suspect this can be written even more elegantly, but didn't want to > rework the code too much. > > I also took the liberty to remove some whitespace. > > > Index: fileio.c > =================================================================== > RCS file: /cvs/src/usr.bin/mg/fileio.c,v > retrieving revision 1.104 > @@ -703,7 +706,7 @@ expandtilde(const char *fn) > struct stat statbuf; > const char *cp; > char user[LOGIN_NAME_MAX], path[NFILEN]; > - char *un, *ret; > + char *ret; > size_t ulen, plen; > > path[0] = '\0'; > @@ -722,21 +725,21 @@ expandtilde(const char *fn) > return (NULL); > return(ret); > } > + pw = getpwuid(geteuid()); > if (ulen == 0) { /* ~/ or ~ */ > - if ((un = getlogin()) != NULL) > - (void)strlcpy(user, un, sizeof(user)); > + if (pw != NULL) > + (void)strlcpy(user, pw->pw_name, sizeof(user)); > else > user[0] = '\0'; > } else { /* ~user/ or ~user */ > memcpy(user, &fn[1], ulen); > user[ulen] = '\0'; > } > - pw = getpwnam(user); > if (pw != NULL) { > plen = strlcpy(path, pw->pw_dir, sizeof(path)); > if (plen == 0 || path[plen - 1] != '/') { > if (strlcat(path, "/", sizeof(path)) >= > sizeof(path)) { > - dobeep(); > + dobeep(); > ewprintf("Path too long"); > return (NULL); > }
not quite, you leave pw unitialized in the else part. Lucas Gabriel Vuotto came up with a similar patch (to a different problem) back in 2017: https://marc.info/?l=openbsd-tech&m=149521389822841&w=2 Which I subsequently slacked on, sorry! Here is a slightly tweaked version of Lucas' diff: - removed now unused un variable - geteuid() instead of getuid() Han, Lucas, does this work for you? diff --git fileio.c fileio.c index 0987f6f30de..339088f5e2d 100644 --- fileio.c +++ fileio.c @@ -703,7 +703,7 @@ expandtilde(const char *fn) struct stat statbuf; const char *cp; char user[LOGIN_NAME_MAX], path[NFILEN]; - char *un, *ret; + char *ret; size_t ulen, plen; path[0] = '\0'; @@ -722,16 +722,13 @@ expandtilde(const char *fn) return (NULL); return(ret); } - if (ulen == 0) { /* ~/ or ~ */ - if ((un = getlogin()) != NULL) - (void)strlcpy(user, un, sizeof(user)); - else - user[0] = '\0'; - } else { /* ~user/ or ~user */ + if (ulen == 0) /* ~/ or ~ */ + pw = getpwuid(geteuid()); + else { /* ~user/ or ~user */ memcpy(user, &fn[1], ulen); user[ulen] = '\0'; + pw = getpwnam(user); } - pw = getpwnam(user); if (pw != NULL) { plen = strlcpy(path, pw->pw_dir, sizeof(path)); if (plen == 0 || path[plen - 1] != '/') { -- I'm not entirely sure you are real.