From: Kevin McCarthy Date: Sat, 6 Jan 2018 04:39:50 +0000 (-0800) Subject: Fix improper signed int conversion of IMAP uid and msn values. X-Git-Tag: mutt-1-9-3-rel~11 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b8190ef365953b525e319bd26c895afa9c8d2a7e;p=mutt Fix improper signed int conversion of IMAP uid and msn values. Several places in the imap code, when parsing "number" and "nz-number" values from the IMAP data, use atoi() and strtol(). This is incorrect, and can result in failures when a uid value happens to be larger than 2^31. Create a helper function, mutt_atoui() and use that instead. One place was using strtol() and relying on the endptr parameter, and so was changed to use strtoul() instead. Thanks to Paul Saunders for the bug report and original patch, which this commit is based on. --- diff --git a/imap/command.c b/imap/command.c index 8529d9de..554aeb26 100644 --- a/imap/command.c +++ b/imap/command.c @@ -33,6 +33,7 @@ #include #include +#include #define IMAP_CMD_BUFSIZE 512 @@ -497,7 +498,7 @@ static int cmd_handle_untagged (IMAP_DATA* idata) dprint (2, (debugfile, "Handling EXISTS\n")); /* new mail arrived */ - count = atoi (pn); + mutt_atoui (pn, &count); if ( !(idata->reopen & IMAP_EXPUNGE_PENDING) && count < idata->max_msn) @@ -615,8 +616,8 @@ static void cmd_parse_expunge (IMAP_DATA* idata, const char* s) dprint (2, (debugfile, "Handling EXPUNGE\n")); - exp_msn = atoi (s); - if (exp_msn < 1 || exp_msn > idata->max_msn) + if (mutt_atoui (s, &exp_msn) < 0 || + exp_msn < 1 || exp_msn > idata->max_msn) return; h = idata->msn_index[exp_msn - 1]; @@ -655,8 +656,8 @@ static void cmd_parse_fetch (IMAP_DATA* idata, char* s) dprint (3, (debugfile, "Handling FETCH\n")); - msn = atoi (s); - if (msn < 1 || msn > idata->max_msn) + if (mutt_atoui (s, &msn) < 0 || + msn < 1 || msn > idata->max_msn) { dprint (3, (debugfile, "FETCH response ignored for this message\n")); return; @@ -669,7 +670,7 @@ static void cmd_parse_fetch (IMAP_DATA* idata, char* s) return; } - dprint (2, (debugfile, "Message UID %d updated\n", HEADER_DATA(h)->uid)); + dprint (2, (debugfile, "Message UID %u updated\n", HEADER_DATA(h)->uid)); /* skip FETCH */ s = imap_next_word (s); s = imap_next_word (s); @@ -701,7 +702,11 @@ static void cmd_parse_fetch (IMAP_DATA* idata, char* s) { s += 3; SKIPWS (s); - uid = (unsigned int) atoi (s); + if (mutt_atoui (s, &uid) < 0) + { + dprint (2, (debugfile, "Illegal UID. Skipping update.\n")); + return; + } if (uid != HEADER_DATA(h)->uid) { dprint (2, (debugfile, "FETCH UID vs MSN mismatch. Skipping update.\n")); @@ -912,7 +917,8 @@ static void cmd_parse_search (IMAP_DATA* idata, const char* s) while ((s = imap_next_word ((char*)s)) && *s != '\0') { - uid = (unsigned int)atoi (s); + if (mutt_atoui (s, &uid) < 0) + continue; h = (HEADER *)int_hash_find (idata->uid_hash, uid); if (h) h->matched = 1; @@ -927,7 +933,7 @@ static void cmd_parse_status (IMAP_DATA* idata, char* s) char* value; BUFFY* inc; IMAP_MBOX mx; - int count; + unsigned int count; IMAP_STATUS *status; unsigned int olduv, oldun; long litlen; @@ -969,7 +975,14 @@ static void cmd_parse_status (IMAP_DATA* idata, char* s) while (*s && *s != ')') { value = imap_next_word (s); - count = strtol (value, &value, 10); + + errno = 0; + count = strtoul (value, &value, 10); + if (errno == ERANGE && count == ULONG_MAX) + { + dprint (1, (debugfile, "Error parsing STATUS number\n")); + return; + } if (!ascii_strncmp ("MESSAGES", s, 8)) { @@ -989,7 +1002,7 @@ static void cmd_parse_status (IMAP_DATA* idata, char* s) if (*s && *s != ')') s = imap_next_word (s); } - dprint (3, (debugfile, "%s (UIDVALIDITY: %d, UIDNEXT: %d) %d messages, %d recent, %d unseen\n", + dprint (3, (debugfile, "%s (UIDVALIDITY: %u, UIDNEXT: %u) %d messages, %d recent, %d unseen\n", status->name, status->uidvalidity, status->uidnext, status->messages, status->recent, status->unseen)); @@ -1028,7 +1041,7 @@ static void cmd_parse_status (IMAP_DATA* idata, char* s) if (value && !imap_mxcmp (mailbox, value)) { - dprint (3, (debugfile, "Found %s in buffy list (OV: %d ON: %d U: %d)\n", + dprint (3, (debugfile, "Found %s in buffy list (OV: %u ON: %u U: %d)\n", mailbox, olduv, oldun, status->unseen)); if (option(OPTMAILCHECKRECENT)) diff --git a/imap/imap.c b/imap/imap.c index b3e780c8..8a089545 100644 --- a/imap/imap.c +++ b/imap/imap.c @@ -266,7 +266,7 @@ void imap_expunge_mailbox (IMAP_DATA* idata) if (h->index == INT_MAX) { - dprint (2, (debugfile, "Expunging message UID %d.\n", HEADER_DATA (h)->uid)); + dprint (2, (debugfile, "Expunging message UID %u.\n", HEADER_DATA (h)->uid)); h->active = 0; idata->ctx->size -= h->content->length; @@ -704,7 +704,8 @@ static int imap_open_mailbox (CONTEXT* ctx) dprint (3, (debugfile, "Getting mailbox UIDVALIDITY\n")); pc += 3; pc = imap_next_word (pc); - idata->uid_validity = strtol (pc, NULL, 10); + if (mutt_atoui (pc, &idata->uid_validity) < 0) + goto fail; status->uidvalidity = idata->uid_validity; } else if (ascii_strncasecmp ("OK [UIDNEXT", pc, 11) == 0) @@ -712,7 +713,8 @@ static int imap_open_mailbox (CONTEXT* ctx) dprint (3, (debugfile, "Getting mailbox UIDNEXT\n")); pc += 3; pc = imap_next_word (pc); - idata->uidnext = strtol (pc, NULL, 10); + if (mutt_atoui (pc, &idata->uidnext) < 0) + goto fail; status->uidnext = idata->uidnext; } else @@ -1751,7 +1753,7 @@ IMAP_STATUS* imap_mboxcache_get (IMAP_DATA* idata, const char* mbox, int create) } status->uidvalidity = *uidvalidity; status->uidnext = uidnext ? *uidnext: 0; - dprint (3, (debugfile, "mboxcache: hcache uidvalidity %d, uidnext %d\n", + dprint (3, (debugfile, "mboxcache: hcache uidvalidity %u, uidnext %u\n", status->uidvalidity, status->uidnext)); } mutt_hcache_free ((void **)&uidvalidity); diff --git a/imap/message.c b/imap/message.c index 4c882165..4caa29e4 100644 --- a/imap/message.c +++ b/imap/message.c @@ -531,7 +531,7 @@ int imap_fetch_message (CONTEXT *ctx, MESSAGE *msg, int msgno) char *pc; long bytes; progress_t progressbar; - int uid; + unsigned int uid; int cacheno; IMAP_CACHE *cache; int read; @@ -618,7 +618,8 @@ int imap_fetch_message (CONTEXT *ctx, MESSAGE *msg, int msgno) if (ascii_strncasecmp ("UID", pc, 3) == 0) { pc = imap_next_word (pc); - uid = atoi (pc); + if (mutt_atoui (pc, &uid) < 0) + goto bail; if (uid != HEADER_DATA(h)->uid) mutt_error (_("The message index is incorrect. Try reopening the mailbox.")); } @@ -1244,7 +1245,8 @@ static int msg_fetch_header (CONTEXT* ctx, IMAP_HEADER* h, char* buf, FILE* fp) /* skip to message number */ buf = imap_next_word (buf); - h->data->msn = atoi (buf); + if (mutt_atoui (buf, &h->data->msn) < 0) + return rc; /* find FETCH tag */ buf = imap_next_word (buf); @@ -1316,7 +1318,8 @@ static int msg_parse_fetch (IMAP_HEADER *h, char *s) { s += 3; SKIPWS (s); - h->data->uid = (unsigned int) atoi (s); + if (mutt_atoui (s, &h->data->uid) < 0) + return -1; s = imap_next_word (s); } @@ -1347,7 +1350,8 @@ static int msg_parse_fetch (IMAP_HEADER *h, char *s) while (isdigit ((unsigned char) *s)) *ptmp++ = *s++; *ptmp = 0; - h->content_length = atoi (tmp); + if (mutt_atol (tmp, &h->content_length) < 0) + return -1; } else if (!ascii_strncasecmp ("BODY", s, 4) || !ascii_strncasecmp ("RFC822.HEADER", s, 13)) diff --git a/lib.c b/lib.c index 224232b8..583d2fff 100644 --- a/lib.c +++ b/lib.c @@ -1083,3 +1083,59 @@ int mutt_atol (const char *str, long *dst) return -1; return 0; } + +/* NOTE: this function's return value breaks with the above three functions. + * The imap code lexes uint values out of a stream of characters without + * tokenization. The above functions return -1 if there is input beyond + * the number. + * + * returns: 1 - successful conversion, with trailing characters + * 0 - successful conversion + * -1 - invalid input + * -2 - input out of range + */ +int mutt_atoui (const char *str, unsigned int *dst) +{ + int rc; + unsigned long res; + unsigned int tmp; + unsigned int *t = dst ? dst : &tmp; + + *t = 0; + + if ((rc = mutt_atoul (str, &res)) < 0) + return rc; + if ((unsigned int) res != res) + return -2; + + *t = (unsigned int) res; + return rc; +} + +/* NOTE: this function's return value is different from mutt_atol. + * + * returns: 1 - successful conversion, with trailing characters + * 0 - successful conversion + * -1 - invalid input + */ +int mutt_atoul (const char *str, unsigned long *dst) +{ + unsigned long r; + unsigned long *res = dst ? dst : &r; + char *e = NULL; + + /* no input: 0 */ + if (!str || !*str) + { + *res = 0; + return 0; + } + + errno = 0; + *res = strtoul (str, &e, 10); + if (*res == ULONG_MAX && errno == ERANGE) + return -1; + if (e && *e != '\0') + return 1; + return 0; +} diff --git a/lib.h b/lib.h index ee85cb90..61ac700d 100644 --- a/lib.h +++ b/lib.h @@ -191,6 +191,8 @@ char *safe_strdup (const char *); int mutt_atos (const char *, short *); int mutt_atoi (const char *, int *); int mutt_atol (const char *, long *); +int mutt_atoui (const char *, unsigned int *); +int mutt_atoul (const char *, unsigned long *); const char *mutt_stristr (const char *, const char *); const char *mutt_basename (const char *);