Hi Greg,
On Jan12, 2011, at 03:32 , Greg Banks wrote:
> On 12/01/11 02:21, Florian Pflug wrote:
>> 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.
I've just upgraded to cyrus 2.4.8, and it seems that the bug is still there.
Or at least I can set neither mine nor your patch in timsieved/parser.c -
in fact, parser.c seems to have hardly changed since 2.4.0 at all.
# git diff cyrus-imapd-2.4.0..cyrus-imapd-2.4.8 timsieved/parser.c
diff --git a/timsieved/parser.c b/timsieved/parser.c
index dd8aaa6..731ce99 100644
--- a/timsieved/parser.c
+++ b/timsieved/parser.c
@@ -444,6 +444,9 @@ int parser(struct protstream *sieved_out, struct protstream
*sieved_in)
goto error;
}
+ /* XXX discard any input pipelined after STARTTLS */
+ prot_flush(sieved_in);
+
if(referral_host)
goto do_referral;
best regards,
Florian Pflug
