Skip to content

Commit 3824e35

Browse files
bengotowclaude
andauthored
Cherry-pick upstream mailcore2 bug fixes and improvements (#16)
* Cherry-pick upstream mailcore2 bug fixes and improvements Cherry-picked the following commits from upstream MailCore/mailcore2: 1. fad23d73 - Ensure checking SSL Certificate in IMAP StartTLS Security fix: adds certificate validation after StartTLS connection to close a potential security gap where certificate validation wasn't being performed in that connection mode. 2. cccebc79 - Fix IMAPMessagesRequestKindFullHeaders handling (#1947) Bug fix: properly handles the FullHeaders flag when fetching messages by UID, fetching complete headers when the flag is set. 3. 29f9488a - Add IMAPMessagesRequestKindAllHeaders flag New feature: adds a new flag to fetch all non-parsed headers, unlike FullHeaders which fetches a limited set. These are the only relevant commits since v0.6.4 (Aug 2020) through Nov 2022 that are bug fixes or functionality improvements for the C++ core. Excluded: Swift/SPM changes, Android changes, Obj-C specific fixes, documentation updates, and Xcode project changes. * Cherry-pick upstream libetpan bug fixes and security patches This commit cherry-picks 8 critical bug fixes and security patches from upstream libetpan (versions 1.9.2-1.9.4) to the vendored libetpan 1.9.1. Patches Applied: 1. CVE-2022-4121 Fix (commit 5c9eb6b) - Fixed crash when st_info_list is NULL in mailimap_mailbox_data_status_free - Prevents NULL pointer dereference vulnerability - File: src/low-level/imap/mailimap_types.c 2. STARTTLS Response Injection Protection - IMAP (commit 1002a01) - Detects extra data after STARTTLS response to prevent response injection attacks - Returns MAILIMAP_ERROR_STARTTLS if extra data is detected - File: src/low-level/imap/mailimap.c 3. STARTTLS Response Injection Protection - SMTP/POP3 (commit 298460a) - Same protection for SMTP and POP3 protocols - Files: src/low-level/smtp/mailsmtp.c, src/low-level/pop3/mailpop3.c 4. Quota SIGSEGV Fix (commit 180b37a) - Fixed SIGSEGV in mailimap_quota_getquotaroot - Properly passes parser context to parser functions instead of NULL - File: src/low-level/imap/quota_parser.c 5. TLS Timeout Fix for GnuTLS (commit 4aee224) - Fixed timeout values for gnutls_handshake_set_timeout (ms vs seconds) - Prevents 1000x shorter timeouts than requested on GnuTLS platforms - File: src/data-types/mailstream_ssl.c 6. IMAP Logout Return Code Fix (commit 27d6f41) - Fixed incorrect error handling when server closes connection after BYE - MAILIMAP_ERROR_STREAM is now expected and treated as success - File: src/low-level/imap/mailimap.c 7. MIME Field Location Handler (commit b4088cb) - Added missing handler for MAILMIME_FIELD_LOCATION in mailmime_write() - File: src/low-level/mime/mailmime_write_generic.c 8. snprintf() Output Fix (commit 1fef3a0) - Fixed incorrect snprintf() usage in test file - File: tests/frm-simple.c Patches Already Included (in commit 01656c8 from Aug 2022): - Buffer overwrite fix for empty strings (commit 078b924) - Memory leak fix in mailimap_hack_date_time_parse (commit 5b43488) - Proper realloc() usage fix (commit 92f8b23) - Resent-Bcc header parsing fix (commit 79eefd5) Patches Excluded: - Platform-specific commits (iOS/macOS/Android build fixes) - Build system changes (pkg-config, configure.ac) - Documentation-only changes - Reverted commits from upstream - Features not present in libetpan 1.9.1 (e.g., clientid, TLS SNI) Analysis Summary: - Base vendored version: libetpan 1.9.1 - Upstream versions analyzed: 1.9.2, 1.9.3, 1.9.4 - Total commits reviewed: ~100 commits since 1.9.1 - Security fixes applied: 3 (CVE-2022-4121, 2x STARTTLS) - Bug fixes applied: 5 - Files modified: 8 files, 89 insertions, 24 deletions This update brings important security fixes and stability improvements to the email sync engine while maintaining compatibility with the existing codebase. * Add vendor library update workflow documentation Documents the process for cherry-picking upstream bug fixes and improvements into vendored dependencies (mailcore2, libetpan) while preserving local modifications. Includes: - Step-by-step workflow - Commit categorization guidelines - Example from mailcore2 update --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 314c5ea commit 3824e35

14 files changed

Lines changed: 234 additions & 28 deletions

File tree

Vendor/libetpan/src/data-types/mailstream_ssl.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -624,7 +624,7 @@ static struct mailstream_ssl_data * ssl_data_new(int fd, time_t timeout,
624624
timeout_value = mailstream_network_delay.tv_sec * 1000 + mailstream_network_delay.tv_usec / 1000;
625625
}
626626
else {
627-
timeout_value = timeout;
627+
timeout_value = timeout * 1000;
628628
}
629629
#if GNUTLS_VERSION_NUMBER >= 0x030100
630630
gnutls_handshake_set_timeout(session, timeout_value);

Vendor/libetpan/src/low-level/imap/mailimap.c

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -749,7 +749,13 @@ int mailimap_logout(mailimap * session)
749749
}
750750

751751
r = mailimap_parse_response(session, &response);
752-
if (r != MAILIMAP_NO_ERROR) {
752+
if (r == MAILIMAP_ERROR_STREAM) {
753+
// the response is expected to be MAILIMAP_ERROR_STREAM
754+
// because the server responds with BYE so the stream
755+
// is immediately closed
756+
res = MAILIMAP_NO_ERROR;
757+
goto close;
758+
} else if (r != MAILIMAP_NO_ERROR) {
753759
res = r;
754760
goto close;
755761
}
@@ -2422,6 +2428,13 @@ int mailimap_starttls(mailimap * session)
24222428

24232429
mailimap_response_free(response);
24242430

2431+
// Detect if the server send extra data after the STARTTLS response.
2432+
// This *may* be a "response injection attack".
2433+
if (session->imap_stream->read_buffer_len != 0) {
2434+
// Since it is also an IMAP protocol violation, exit.
2435+
return MAILIMAP_ERROR_STARTTLS;
2436+
}
2437+
24252438
switch (error_code) {
24262439
case MAILIMAP_RESP_COND_STATE_OK:
24272440
return MAILIMAP_NO_ERROR;

Vendor/libetpan/src/low-level/imap/mailimap_types.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1389,9 +1389,11 @@ void
13891389
mailimap_mailbox_data_status_free(struct mailimap_mailbox_data_status * info)
13901390
{
13911391
mailimap_mailbox_free(info->st_mailbox);
1392-
clist_foreach(info->st_info_list, (clist_func) mailimap_status_info_free,
1393-
NULL);
1394-
clist_free(info->st_info_list);
1392+
if (info->st_info_list != NULL) {
1393+
clist_foreach(info->st_info_list, (clist_func) mailimap_status_info_free,
1394+
NULL);
1395+
clist_free(info->st_info_list);
1396+
}
13951397
free(info);
13961398
}
13971399

Vendor/libetpan/src/low-level/imap/quota_parser.c

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ mailimap_quota_quota_resource_parse(mailstream * fd, MMAPString * buffer, struct
103103
}
104104

105105
static int
106-
mailimap_quota_quota_list_nonempty_parse(mailstream * fd, MMAPString * buffer,
106+
mailimap_quota_quota_list_nonempty_parse(mailstream * fd, MMAPString * buffer, struct mailimap_parser_context * parser_ctx,
107107
size_t * indx, clist ** result,
108108
size_t progr_rate, progress_function * progr_fun)
109109
{
@@ -114,13 +114,13 @@ mailimap_quota_quota_list_nonempty_parse(mailstream * fd, MMAPString * buffer,
114114

115115
cur_token = * indx;
116116

117-
r = mailimap_oparenth_parse(fd, buffer, NULL, &cur_token);
117+
r = mailimap_oparenth_parse(fd, buffer, parser_ctx, &cur_token);
118118
if (r != MAILIMAP_NO_ERROR) {
119119
res = r;
120120
goto err;
121121
}
122122

123-
r = mailimap_struct_spaced_list_parse(fd, buffer, NULL,
123+
r = mailimap_struct_spaced_list_parse(fd, buffer, parser_ctx,
124124
&cur_token, &quota_resource_list,
125125
&mailimap_quota_quota_resource_parse,
126126
(mailimap_struct_destructor *)
@@ -131,7 +131,7 @@ mailimap_quota_quota_list_nonempty_parse(mailstream * fd, MMAPString * buffer,
131131
goto err;
132132
}
133133

134-
r = mailimap_cparenth_parse(fd, buffer, NULL, &cur_token);
134+
r = mailimap_cparenth_parse(fd, buffer, parser_ctx, &cur_token);
135135
if (r != MAILIMAP_NO_ERROR) {
136136
res = r;
137137
goto quota_list_free;
@@ -151,7 +151,7 @@ mailimap_quota_quota_list_nonempty_parse(mailstream * fd, MMAPString * buffer,
151151
}
152152

153153
static int
154-
mailimap_quota_quota_list_empty_parse(mailstream * fd, MMAPString * buffer,
154+
mailimap_quota_quota_list_empty_parse(mailstream * fd, MMAPString * buffer, struct mailimap_parser_context * parser_ctx,
155155
size_t * indx, clist ** result,
156156
size_t progr_rate, progress_function * progr_fun)
157157
{
@@ -161,12 +161,12 @@ mailimap_quota_quota_list_empty_parse(mailstream * fd, MMAPString * buffer,
161161

162162
cur_token = * indx;
163163

164-
r = mailimap_oparenth_parse(fd, buffer, NULL, &cur_token);
164+
r = mailimap_oparenth_parse(fd, buffer, parser_ctx, &cur_token);
165165
if (r != MAILIMAP_NO_ERROR) {
166166
return r;
167167
}
168168

169-
r = mailimap_cparenth_parse(fd, buffer, NULL, &cur_token);
169+
r = mailimap_cparenth_parse(fd, buffer, parser_ctx, &cur_token);
170170
if (r != MAILIMAP_NO_ERROR) {
171171
return r;
172172
}
@@ -183,24 +183,24 @@ mailimap_quota_quota_list_empty_parse(mailstream * fd, MMAPString * buffer,
183183
}
184184

185185
static int
186-
mailimap_quota_quota_list_parse(mailstream * fd, MMAPString * buffer,
186+
mailimap_quota_quota_list_parse(mailstream * fd, MMAPString * buffer, struct mailimap_parser_context * parser_ctx,
187187
size_t * indx, clist ** result,
188188
size_t progr_rate, progress_function * progr_fun)
189189
{
190190
int r;
191191

192-
r = mailimap_quota_quota_list_empty_parse(fd, buffer, indx, result,
192+
r = mailimap_quota_quota_list_empty_parse(fd, buffer, parser_ctx, indx, result,
193193
progr_rate, progr_fun);
194194
if (r == MAILIMAP_NO_ERROR) {
195195
return r;
196196
}
197197

198-
return mailimap_quota_quota_list_nonempty_parse(fd, buffer, indx, result,
198+
return mailimap_quota_quota_list_nonempty_parse(fd, buffer, parser_ctx, indx, result,
199199
progr_rate, progr_fun);
200200
}
201201

202202
static int
203-
mailimap_quota_quota_response_parse(mailstream * fd, MMAPString * buffer,
203+
mailimap_quota_quota_response_parse(mailstream * fd, MMAPString * buffer, struct mailimap_parser_context * parser_ctx,
204204
size_t * indx, struct mailimap_quota_quota_data ** result,
205205
size_t progr_rate, progress_function * progr_fun)
206206
{
@@ -226,7 +226,7 @@ mailimap_quota_quota_response_parse(mailstream * fd, MMAPString * buffer,
226226
goto err;
227227
}
228228

229-
r = mailimap_astring_parse(fd, buffer, NULL, &cur_token, &quotaroot,
229+
r = mailimap_astring_parse(fd, buffer, parser_ctx, &cur_token, &quotaroot,
230230
progr_rate, progr_fun);
231231
if (r != MAILIMAP_NO_ERROR) {
232232
res = r;
@@ -239,7 +239,7 @@ mailimap_quota_quota_response_parse(mailstream * fd, MMAPString * buffer,
239239
goto quotaroot_free;
240240
}
241241

242-
r = mailimap_quota_quota_list_parse(fd, buffer, &cur_token,
242+
r = mailimap_quota_quota_list_parse(fd, buffer, parser_ctx, &cur_token,
243243
&quota_list, progr_rate, progr_fun);
244244
if (r != MAILIMAP_NO_ERROR) {
245245
res = r;
@@ -268,7 +268,7 @@ mailimap_quota_quota_response_parse(mailstream * fd, MMAPString * buffer,
268268
}
269269

270270
static int
271-
mailimap_quota_quotaroot_response_parse(mailstream * fd, MMAPString * buffer,
271+
mailimap_quota_quotaroot_response_parse(mailstream * fd, MMAPString * buffer, struct mailimap_parser_context * parser_ctx,
272272
size_t * indx, struct mailimap_quota_quotaroot_data ** result,
273273
size_t progr_rate, progress_function * progr_fun)
274274
{
@@ -295,7 +295,7 @@ mailimap_quota_quotaroot_response_parse(mailstream * fd, MMAPString * buffer,
295295
goto err;
296296
}
297297

298-
r = mailimap_mailbox_parse(fd, buffer, NULL, &cur_token, &mailbox,
298+
r = mailimap_mailbox_parse(fd, buffer, parser_ctx, &cur_token, &mailbox,
299299
progr_rate, progr_fun);
300300
if (r != MAILIMAP_NO_ERROR) {
301301
res = r;
@@ -317,7 +317,7 @@ mailimap_quota_quotaroot_response_parse(mailstream * fd, MMAPString * buffer,
317317
goto quotaroot_list_free;
318318
}
319319

320-
r = mailimap_astring_parse(fd, buffer, NULL, &cur_token, &quotaroot,
320+
r = mailimap_astring_parse(fd, buffer, parser_ctx, &cur_token, &quotaroot,
321321
progr_rate, progr_fun);
322322
if (r != MAILIMAP_NO_ERROR) {
323323
res = r;
@@ -386,15 +386,15 @@ int mailimap_quota_parse(int calling_parser, mailstream * fd,
386386
switch (calling_parser)
387387
{
388388
case MAILIMAP_EXTENDED_PARSER_MAILBOX_DATA:
389-
r = mailimap_quota_quota_response_parse(fd, buffer, indx,
389+
r = mailimap_quota_quota_response_parse(fd, buffer, parser_ctx, indx,
390390
&quota_data, progr_rate, progr_fun);
391391
if (r == MAILIMAP_NO_ERROR) {
392392
type = MAILIMAP_QUOTA_TYPE_QUOTA_DATA;
393393
data = quota_data;
394394
}
395395

396396
if (r == MAILIMAP_ERROR_PARSE) {
397-
r = mailimap_quota_quotaroot_response_parse(fd, buffer, indx,
397+
r = mailimap_quota_quotaroot_response_parse(fd, buffer, parser_ctx, indx,
398398
&quotaroot_data, progr_rate, progr_fun);
399399
if (r == MAILIMAP_NO_ERROR) {
400400
type = MAILIMAP_QUOTA_TYPE_QUOTAROOT_DATA;

Vendor/libetpan/src/low-level/mime/mailmime_write_generic.c

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,9 @@ static int mailmime_version_write_driver(int (* do_write)(void *, const char *,
8181
static int mailmime_encoding_write_driver(int (* do_write)(void *, const char *, size_t), void * data, int * col,
8282
struct mailmime_mechanism * encoding);
8383

84+
static int mailmime_location_write_driver(int (* do_write)(void *, const char *, size_t), void *data, int *col,
85+
char *location);
86+
8487
static int mailmime_language_write_driver(int (* do_write)(void *, const char *, size_t), void * data, int * col,
8588
struct mailmime_language * language);
8689

@@ -169,6 +172,10 @@ static int mailmime_field_write_driver(int (* do_write)(void *, const char *, si
169172
r = mailmime_language_write_driver(do_write, data, col, field->fld_data.fld_language);
170173
break;
171174

175+
case MAILMIME_FIELD_LOCATION:
176+
r = mailmime_location_write_driver(do_write, data, col, field->fld_data.fld_location);
177+
break;
178+
172179
default:
173180
r = MAILIMF_ERROR_INVAL;
174181
break;
@@ -310,6 +317,33 @@ static int mailmime_encoding_write_driver(int (* do_write)(void *, const char *,
310317
return MAILIMF_NO_ERROR;
311318
}
312319

320+
static int mailmime_location_write_driver(int (* do_write)(void *, const char *, size_t), void *data, int *col,
321+
char *location)
322+
{
323+
int r;
324+
int len = strlen(location);
325+
326+
r = mailimf_string_write_driver(do_write, data, col, "Content-Location: ", 18);
327+
if (r != MAILIMF_NO_ERROR)
328+
return r;
329+
330+
if (*col > 1 && *col + len > MAX_MAIL_COL) {
331+
r = mailimf_string_write_driver(do_write, data, col, "\r\n ", 3);
332+
if (r != MAILIMF_NO_ERROR)
333+
return r;
334+
}
335+
336+
r = mailimf_string_write_driver(do_write, data, col, location, len);
337+
if (r != MAILIMF_NO_ERROR)
338+
return r;
339+
340+
r = mailimf_string_write_driver(do_write, data, col, "\r\n", 2);
341+
if (r != MAILIMF_NO_ERROR)
342+
return r;
343+
344+
return MAILIMF_NO_ERROR;
345+
}
346+
313347
static int mailmime_language_write_driver(int (* do_write)(void *, const char *, size_t), void * data, int * col,
314348
struct mailmime_language * language)
315349
{

Vendor/libetpan/src/low-level/pop3/mailpop3.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -959,6 +959,14 @@ int mailpop3_stls(mailpop3 * f)
959959

960960
if (r != RESPONSE_OK)
961961
return MAILPOP3_ERROR_STLS_NOT_SUPPORTED;
962+
963+
// Detect if the server send extra data after the STLS response.
964+
// This *may* be a "response injection attack".
965+
if (f->pop3_stream->read_buffer_len != 0) {
966+
// Since it is also protocol violation, exit.
967+
// There is no error type for STARTTLS errors in POP3
968+
return MAILPOP3_ERROR_SSL;
969+
}
962970

963971
return MAILPOP3_NO_ERROR;
964972
}

Vendor/libetpan/src/low-level/smtp/mailsmtp.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,6 +1121,14 @@ int mailesmtp_starttls(mailsmtp * session)
11211121
return MAILSMTP_ERROR_STREAM;
11221122
r = read_response(session);
11231123

1124+
// Detect if the server send extra data after the STARTTLS response.
1125+
// This *may* be a "response injection attack".
1126+
if (session->stream->read_buffer_len != 0) {
1127+
// Since it is also protocol violation, exit.
1128+
// There is no general error type for STARTTLS errors in SMTP
1129+
return MAILSMTP_ERROR_SSL;
1130+
}
1131+
11241132
switch (r) {
11251133
case 220:
11261134
return MAILSMTP_NO_ERROR;

Vendor/libetpan/tests/frm-simple.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ static void simple_print_mail_info(mailmessage * msg)
5959

6060
strip_crlf(dsp_subject);
6161

62-
snprintf(output, MAX_OUTPUT, "%3i: %-21.21s %-53.53s\n",
62+
snprintf(output, MAX_OUTPUT, "%3i: %-21.21s %-52.52s\n",
6363
msg->msg_index % 1000, dsp_from, dsp_subject);
6464

6565
printf("%s\n", output);

Vendor/mailcore2/src/core/abstract/MCMessageConstants.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,14 @@ namespace mailcore {
7070
IMAPMessagesRequestKindHeaders = 1 << 1,
7171
IMAPMessagesRequestKindStructure = 1 << 2,
7272
IMAPMessagesRequestKindInternalDate = 1 << 3,
73-
IMAPMessagesRequestKindFullHeaders = 1 << 4,
73+
IMAPMessagesRequestKindFullHeaders = 1 << 4, //Fetches the non-parsed list of a limited set of headers.
7474
IMAPMessagesRequestKindHeaderSubject = 1 << 5,
7575
IMAPMessagesRequestKindGmailLabels = 1 << 6,
7676
IMAPMessagesRequestKindGmailMessageID = 1 << 7,
7777
IMAPMessagesRequestKindGmailThreadID = 1 << 8,
7878
IMAPMessagesRequestKindExtraHeaders = 1 << 9,
7979
IMAPMessagesRequestKindSize = 1 << 10,
80+
IMAPMessagesRequestKindAllHeaders = 1 << 11, // Unlike Full headers this will fetch all the non-parsed headers
8081
};
8182

8283
enum IMAPFetchRequestType {

Vendor/mailcore2/src/core/imap/MCIMAPSession.cpp

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -672,18 +672,24 @@ void IMAPSession::connect(ErrorCode * pError)
672672
* pError = ErrorTLSNotAvailable;
673673
goto close;
674674
}
675+
if (!checkCertificate()) {
676+
MCLog("StartTLS ssl connect certificate ERROR %d", r);
677+
* pError = ErrorCertificate;
678+
goto close;
679+
}
680+
675681
break;
676682

677683
case ConnectionTypeTLS:
678684
r = mailimap_ssl_connect_voip(mImap, MCUTF8(mHostname), mPort, isVoIPEnabled());
679-
MCLog("ssl connect %s %u %u", MCUTF8(mHostname), mPort, r);
685+
MCLog("TLS ssl connect %s %u %u", MCUTF8(mHostname), mPort, r);
680686
if (hasError(r)) {
681687
MCLog("connect error %i", r);
682688
* pError = ErrorConnection;
683689
goto close;
684690
}
685691
if (!checkCertificate()) {
686-
MCLog("ssl connect certificate ERROR %d", r);
692+
MCLog("TLS ssl connect certificate ERROR %d", r);
687693
* pError = ErrorCertificate;
688694
goto close;
689695
}
@@ -2611,7 +2617,17 @@ IMAPSyncResult * IMAPSession::fetchMessages(String * folder, IMAPMessagesRequest
26112617
struct mailimap_section * section;
26122618

26132619
imap_hdrlist = mailimap_header_list_new(hdrlist);
2614-
section = mailimap_section_new_header_fields(imap_hdrlist);
2620+
2621+
/*
2622+
* If the IMAPRequestKindFullHeaders is set then we have to fetch the full headers
2623+
* otherwise only the specified list of headers
2624+
*/
2625+
if ((requestKind & IMAPMessagesRequestKindAllHeaders) != 0) {
2626+
section = mailimap_section_new_header();
2627+
} else {
2628+
section = mailimap_section_new_header_fields(imap_hdrlist);
2629+
}
2630+
26152631
fetch_att = mailimap_fetch_att_new_body_peek_section(section);
26162632
mailimap_fetch_type_new_fetch_att_list_add(fetch_type, fetch_att);
26172633
needsHeader = true;

0 commit comments

Comments
 (0)