Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MAILBOXID not alpha/digit and crashes clients #4944

Open
the-djmaze opened this issue Jun 24, 2024 · 2 comments
Open

MAILBOXID not alpha/digit and crashes clients #4944

the-djmaze opened this issue Jun 24, 2024 · 2 comments

Comments

@the-djmaze
Copy link

the-djmaze commented Jun 24, 2024

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

See the-djmaze/snappymail#1640 (comment)

Issue is at

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

Probably caused by

cyrus-imapd/imap/mailbox.c

Lines 1130 to 1135 in 748cfaf

EXPORTED const char *mailbox_uniqueid(const struct mailbox *mailbox)
{
mbentry_t *mbentry = mailbox->mbentry;
return mbentry->uniqueid ? mbentry->uniqueid : mailbox->h.uniqueid;
}

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
Copy link
Contributor

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
Copy link

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants