Subject: Bad interaction of mymailboxdir, dorc and env_init

From: Josko Plazonic <[email-masked]>
Date: Wed, 16 Apr 2003 12:19:26 -0400

Hi,

I was trying to use conf settings in /etc/c-client.conf like
set new-folder-format same-as-inbox
when I noticed that this suddenly broke all access to email in user home 
dir.  It turnes out that myMailboxDir gets set to empty string rather 
than appropriate home dir.

Why:
The reason is that when dorc is called to process config file, it is 
called from env_init before, later in the env_init, myHomeDir is 
properly initialized.  If then one uses config settings like "set 
new-folder-format", they need to access users home directory and hence 
invoke mymailboxdir.  mymailboxdir will then incorrectly set 
myMailboxdir to "", since myHomeDir is such.  myMailboxdir will be then 
on be remembered as such.

There are other configuration setting that rely on myHomeDir being 
properly setup.  Unfortunately, myHomeDir depends on c-client.conf as 
well (e.g. use of blackbox or other settings).  I guess noone ever 
thought of using this command in a non user conf file... If there is no 
desire to modify this behaviour, it should at least be documented ("safe 
to use only in user config file").

I include a sample patch for env_unix.c that solves the problem for me - 
by rerunning dorc another time after myHomeDir was read and by making 
sure mymailboxdir will not set myMailboxdir if myhomedir returns "".

Thanks,

Josko P.
--- imap-2002b/src/osdep/unix/env_unix.c-original	2003-04-16 11:41:53.000000000 -0400
+++ imap-2002b/src/osdep/unix/env_unix.c	2003-04-16 11:43:24.000000000 -0400
@@ -678,6 +678,7 @@
     myHomeDir = cpystr (home);	/* set home directory */
   }
 
+  dorc (NIL,NIL);		/* do systemwide configuration again*/
   if (allowuserconfig) {	/* allow user config files */
     dorc (strcat (strcpy (tmp,myHomeDir),"/.mminit"),T);
     dorc (strcat (strcpy (tmp,myHomeDir),"/.imaprc"),NIL);
@@ -773,7 +774,7 @@
 static char *mymailboxdir ()
 {
   char *home = myhomedir ();
-  if (!myMailboxDir && home) {	/* initialize if first time */
+  if (!myMailboxDir && home && *home) {	/* initialize if first time */
     if (mailsubdir) {
       char tmp[MAILTMPLEN];
       sprintf (tmp,"%s/%s",home,mailsubdir);

---

From: Mark Crispin <[email-masked]>
Date: Wed, 16 Apr 2003 12:36:32 -0700 (PDT)

Unfortunately, your patch breaks other things related to those config
files; dorc() was once called at that point for systemwide config.  It
*is* documented that the config files work in mysterious ways "at your own
risk."

Also, there is no intention that "set new-folder-format same-as-inbox"
will work reasonably in the system configuration file.

I agree that mymailboxdir() should not set myMailboxDir if myHomeDir is
not set up.  However, your fix isn't quite right.

The correct fix is to make that "if" statement check myHomeDir instead of
home:
				/* initialize if first time */
  if (!myMailboxDir && myHomeDir) {

-- Mark --

http://staff.washington.edu/mrc
Science does not emerge from voting, party politics, or public debate.

---

From: Josko Plazonic <[email-masked]>
Date: Wed, 16 Apr 2003 17:24:34 -0400

Hi,

I never intended to present a good solution - just a quick fix that 
worked for me, especially because I also disliked to call dorc again but 
the proper fix was probably too invasive (split stricly configs into 2 
groups, one for systemwide and one for user ones, it is almost there 
already, plus either a boolean flag or maybe invoke once dorc with 
dorc(NIL,NIL) and 2nd time with dorc(SYSCONFIG,NIL)).  If you are 
willing to consider it I am willing to give it a try. 

Anyway, I did read through dorc and it seemed like nothing bad can 
happen - at worst some memory waste in allocating more space for 
strings, which is not too bad as the dorc is run only a limited number 
of times.  I am not being totally reckless :). Anything in particular 
you have in mind that could break (before I go live with it) or is this 
just a generic warning like the one in imaprc.txt?

BTW I avoided myHomeDir only to stay in the spirit, after all, why do 
home=myhomedir(), why just not use myHomeDir directly throughout that 
function?

Finally, some config setttings, and new-folder-format is one of them, 
are really useful at site level... Then again, I do know that there is 
no convincing you of this.  I did read imaprc.txt, know you feelings 
about it, given that wu-imapd is a great piece of free software how can 
I complain - I know how to change it myself, so no problems...

Thanks,

Josko P.

P.S.  I do also have, maybe a 20 lines or so, patch for adding another 
config option that makes mailboxfile fn verify if the file user is 
attempting to open is owned by the user (a trivial addition of another 
option and a stat call).  It helps a lot if you cannot allow access to 
mailserver, in particular where restrict-mailbox-access doesn't help, 
i.e. with symlinks like to /etc/passwd for folder or /etc or /sbin for 
imap folder browsing.  Any takers (as an idea)?

---

From: Mark Crispin <[email-masked]>
Date: Wed, 16 Apr 2003 16:23:19 -0700 (PDT)

On Wed, 16 Apr 2003, Josko Plazonic wrote:
> happen

I forget the details of what went wrong when there was an extra call to
dorc() for the systemwide file at that.  In general, I try to avoid
hacking anything to do with dorc(); it seems that every time that routine
is touched, something gets broken.

> home=myhomedir(), why just not use myHomeDir directly throughout that

The call to mymailboxdir() could happen when myHomeDir isn't initialized,
but a call to myhomedir() is.  Thus, the correct thing to do is to call
myhomedir() first, then checking to see if myHomeDir got set, and if it
did not get set, then don't set myMailboxDir and just return the empty
string.

Your patch covered that case, but also covered the case when the home
directory was root.  You don't want that, particularly in a chroot()
server environment.

> no convincing you of this.

Basically, as noted above, it's because that config file functionality is
fundamentally broken.  It was written in haste years ago to accomplish a
specific purpose, but to be done right requires a much more thorough check
of the values being set and the implications of setting them at any one
time.  As matters stand now, only the minimum to ensure that there isn't a
security weakness is done.

dorc(), and when it is called, needs a complete rewrite.  I'd rather
remove it entirely, except that there are a few sites that depend upon it.

> imap folder browsing.

I don't see how restrictBox doesn't help.

How did the symlinks get there?  imapd doesn't create them.

If the user has shell access to create the symlinks, then why does it
matter?

-- Mark --