cyrusimap / cyrus-imapd

Cyrus IMAP is an email, contacts and calendar server
http://cyrusimap.org
Other
522 stars 145 forks source link

MAILBOXID not alpha/digit and crashes clients #4944

Open the-djmaze opened 1 week ago

the-djmaze commented 1 week ago

There's a bug in OBJECTID RFC 8474 implementation that returns invalid characters in the MAILBOXID.

See https://github.com/the-djmaze/snappymail/issues/1640#issuecomment-2186796033

Issue is at https://github.com/cyrusimap/cyrus-imapd/blob/748cfafef811038e79a8fdc9fd1bb66b6afb4492/imap/imapd.c#L9638

Probably caused by https://github.com/cyrusimap/cyrus-imapd/blob/748cfafef811038e79a8fdc9fd1bb66b6afb4492/imap/mailbox.c#L1130-L1135

JMAP mailboxIds doesn't seem to be affected as they are encoded there.

So maybe use something like

prot_printf(imapd_out, "%cMAILBOXID (0x%x)", sepchar, sd->mailboxid); 

or something like

struct buf buf = BUF_INITIALIZER;
charset_encode(&buf, sd->mailboxid, strlen(sd->mailboxid), ENCODING_BASE64URL);
prot_printf(imapd_out, "%cMAILBOXID (%s)", sepchar, buf_cstring(&buf)); 
buf_free(&buf);

Better approach would be to make uniqueid not binary, because then there's no need for conversions at runtime all the time. But that would probably be a big change?

ksmurchison commented 2 days ago

I don't think that a MAILBOXID (uniqueid) should ever contain non-ASCII chars. @brong can you think of any way that this could happen, outside of corruption?

realsimix commented 2 days ago

BTW, this was on a test server with cyrus-imapd-3.4.1 as shipped with RHEL9. Mail spool was from a 2.4 server, I ran reconstruct and reconstruct -V max and it seemed to be fine. The server then worked without issues in my tests. Could it be that there was a bug in 3.4.1 which was fixed since then?