Bug: OpenSSH (port.) daemon sets $MAIL incorrectly on Solaris (others?)

[Dan Astoorian]

This bug may be specific to the portability releases of OpenSSH.  It's
minor, but trivially corrected.

I'm observing it under:
    * openssh-2.3.0p1 (openssl-0.9.6)
    * compiled under Solaris 2.5.1
    * running under Solaris 2.5.1 and Solaris 8

The relevant source code does not appear to have changed up to and
including openssh-2.9p2.

The bug is that the $MAIL environment variable set by the server
contains an extraneous slash between the spool directory and the userid;
i.e., "/var/mail//djast" instead of "/var/mail/djast" .  Although this
is usually harmless, since the pathname is still valid with the null
pathname component, some of my users are reporting that the extra slash
confuses rmail under emacs in such a way that it will not read the
mailbox.

The cause of the bug is that "defines.h" includes <maillock.h>, and
under Solaris (and possibly other platforms on which HAVE_MAILLOCK_H is
defined), /usr/include/maillock.h defines MAILDIR in a manner which
includes the trailing slash:

    #define MAILDIR         "/var/mail/"
    ...
    #if !defined(_PATH_MAILDIR) && defined(MAILDIR)
    # define _PATH_MAILDIR MAILDIR
    #endif /* !defined(_PATH_MAILDIR) && defined(MAILDIR) */

In session.c, this definition gets used to set the MAIL environment
variable:

	    snprintf(buf, sizeof buf, "%.200s/%.50s",
		     _PATH_MAILDIR, pw->pw_name);
	    child_set_env(&env, &envsize, "MAIL", buf);

This results in the doubled slash.

I can recommend two alternatives to correct the problem.

One trivial workaround for the bug is simply to test whether the last
character of MAILDIR is a slash; e.g.:

	    snprintf(buf, sizeof buf, "%.200s%.1s%.50s",
		     _PATH_MAILDIR,
		     (_PATH_MAILDIR[strlen(_PATH_MAILDIR)-1]=='/')?"":"/",
		     pw->pw_name);
	    child_set_env(&env, &envsize, "MAIL", buf);

A different approach to the problem would be to modify the definitions
so that the trailing slash is always present in the macro; for instance,
by changing "defines.h" to read:

    #ifndef MAIL_DIRECTORY
    # define MAIL_DIRECTORY "/var/spool/mail/"
    #endif

    #ifndef _PATH_MAILDIR
    # define _PATH_MAILDIR MAIL_DIRECTORY
    #endif

    #if !defined(MAILDIR) && defined(_PATH_MAILDIR)
    # define MAILDIR _PATH_MAILDIR "/"
    #endif /* !defined(MAILDIR) && defined(_PATH_MAILDIR) */

and changing the original snprintf() in session.c to user "MAILDIR" in
place of "_PATH_MAILDIR," and removing the slash from the format string.

Note, however, that the second proposed solution is riskier, in that it
makes the (possibly unfounded) assumption that if MAILDIR is defined on
a platform, it will always have a trailing slash, but that _PATH_MAILDIR
never will; the first solution adapts to whatever is provided.

Please let me know if you require further information.

Thanks,

-- 
Dan Astoorian               People shouldn't think that it's better to have
Sysadmin, CSLab             loved and lost than never loved at all.  It's
djast at cs.toronto.edu        not, it's better to have loved and won.  All
www.cs.toronto.edu/~djast/  the other options really suck.    --Dan Redican