On Jan12, 2011, at 03:32 , Greg Banks wrote: > On 12/01/11 02:21, Florian Pflug wrote: >> If the authenticating user is an admin, we proceed even if the mailbox >> lookup fails. In this case, the mboxlist_lookup seems to leave the >> mboxlist_entry uninitialized, making the code believe that the mailbox >> is remote if the bit MBTYPE_REMOTE happens to be set in mbentry.mvtype. >> The crash then happens when xstrdup tried to copy mbentry.partition. >> >> Initializing mbentry to zero in cmd_authenticate() fixes the bug and >> allows admins without mailboxes (like root) to authenticate again >> on my system. > > Thanks, your analysis is correct, but I think a better fix might be the > attached (untested) patch.
That's exactly what I did initially :-) I didn't like it much, though. The bug was probably introduced precisely because someone *didn't* realize that mbentry is uninitialized in the corner-case of an admin user without a mailbox. Leaving things that way carries a high risk of a similar bug being re-introduces by the next one who touches this code. What about doing both the "!r" check and zero-initializing mbentry? > Bron changed the mboxlist_lookup() API very recently (not yet in a release), > which means that patch won't apply to ToT, but the bug is still there. As > coincidentally I touched that code yesterday, I'll fix it. Cool, thanks! best regards, Florian Pflug
