Yes this also works for me.
On Thu, Apr 12, 2018 at 4:52 PM, Florian Obser <flor...@openbsd.org> wrote:
> 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.
>
