G'day Florian,
On 12/01/11 02:21, Florian Pflug wrote:
Hi
I've just found a bug in timsieved's cmd_authenticate on cyrus 2.4.6.
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.
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.
--
Greg.
commit a19110e73a9df20969d9763e0ceff2e1fb992c32
Author: Greg Banks <g...@fastmail.fm>
Date: Wed Jan 12 13:29:47 2011 +1100
timsieved: fix crash in cmd_authenticate
Found and analysed by Florian Pflug. If the mboxlist_lookup() fails, the
uninitialised mbentry is examined and decisions made based on random
stack garbage.
diff --git a/timsieved/parser.c b/timsieved/parser.c
index dd8aaa6..1e16e80 100644
--- a/timsieved/parser.c
+++ b/timsieved/parser.c
@@ -714,7 +714,7 @@ static int cmd_authenticate(struct protstream *sieved_out,
goto cleanup;
}
- if(mbentry.mbtype & MBTYPE_REMOTE) {
+ if (!r && mbentry.mbtype & MBTYPE_REMOTE) {
/* It's a remote mailbox */
if (config_getswitch(IMAPOPT_SIEVE_ALLOWREFERRALS)) {
/* We want to set up a referral */
