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 8bfc4c81aaf99b5ed0ed68ce3a8b059e7beeaa05..d8ffbcaefce27c13848c9342b92b63f3f684f615 100644 (file)
--- a/imap/command.c
+++ b/imap/command.c
@@ -40,6 +40,7 @@
 
 #include "config.h"
 #include <ctype.h>
+#include <errno.h>
 #include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -253,8 +254,7 @@ static void cmd_parse_expunge(struct ImapData *idata, const char *s)
 
   mutt_debug(2, "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];
@@ -299,8 +299,7 @@ static void cmd_parse_fetch(struct ImapData *idata, char *s)
 
   mutt_debug(3, "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)
   {
     mutt_debug(3, "#1 FETCH response ignored for this message\n");
     return;
@@ -313,7 +312,7 @@ static void cmd_parse_fetch(struct ImapData *idata, char *s)
     return;
   }
 
-  mutt_debug(2, "Message UID %d updated\n", HEADER_DATA(h)->uid);
+  mutt_debug(2, "Message UID %u updated\n", HEADER_DATA(h)->uid);
   /* skip FETCH */
   s = imap_next_word(s);
   s = imap_next_word(s);
@@ -346,7 +345,11 @@ static void cmd_parse_fetch(struct ImapData *idata, char *s)
     {
       s += 3;
       SKIPWS(s);
-      uid = (unsigned int) atoi(s);
+      if (mutt_atoui(s, &uid) < 0)
+      {
+        mutt_debug(2, "Illegal UID.  Skipping update.\n");
+        return;
+      }
       if (uid != HEADER_DATA(h)->uid)
       {
         mutt_debug(2, "FETCH UID vs MSN mismatch.  Skipping update.\n");
@@ -610,7 +613,8 @@ static void cmd_parse_search(struct ImapData *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 = (struct Header *) mutt_hash_int_find(idata->uid_hash, uid);
     if (h)
       h->matched = true;
@@ -631,7 +635,7 @@ static void cmd_parse_status(struct ImapData *idata, char *s)
   char *value = NULL;
   struct Buffy *inc = NULL;
   struct ImapMbox mx;
-  int count;
+  unsigned int count;
   struct ImapStatus *status = NULL;
   unsigned int olduv, oldun;
   long litlen;
@@ -673,7 +677,14 @@ static void cmd_parse_status(struct ImapData *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)
+    {
+      mutt_debug(1, "Error parsing STATUS number\n");
+      return;
+    }
 
     if (mutt_str_strncmp("MESSAGES", s, 8) == 0)
     {
@@ -695,7 +706,7 @@ static void cmd_parse_status(struct ImapData *idata, char *s)
   }
   mutt_debug(
       3,
-      "%s (UIDVALIDITY: %d, UIDNEXT: %d) %d messages, %d recent, %d unseen\n",
+      "%s (UIDVALIDITY: %u, UIDNEXT: %u) %d messages, %d recent, %d unseen\n",
       status->name, status->uidvalidity, status->uidnext, status->messages,
       status->recent, status->unseen);
 
@@ -733,7 +744,7 @@ static void cmd_parse_status(struct ImapData *idata, char *s)
 
       if (value && (imap_mxcmp(mailbox, value) == 0))
       {
-        mutt_debug(3, "Found %s in buffy list (OV: %d ON: %d U: %d)\n", mailbox,
+        mutt_debug(3, "Found %s in buffy list (OV: %u ON: %u U: %d)\n", mailbox,
                    olduv, oldun, status->unseen);
 
         if (MailCheckRecent)
@@ -827,7 +838,7 @@ static int cmd_handle_untagged(struct ImapData *idata)
       mutt_debug(2, "Handling EXISTS\n");
 
       /* new mail arrived */
-      count = atoi(pn);
+      mutt_atoui(pn, &count);
 
       if (!(idata->reopen & IMAP_EXPUNGE_PENDING) && count < idata->max_msn)
       {

diff --git a/imap/imap.c b/imap/imap.c
index 77f18f7ef43e205d771f9183d1832b4a68b7dce2..85f9e2c2cbd065c1ec44485294b2175a013f997e 100644 (file)
--- a/imap/imap.c
+++ b/imap/imap.c
@@ -883,7 +883,7 @@ void imap_expunge_mailbox(struct ImapData *idata)
 
     if (h->index == INT_MAX)
     {
-      mutt_debug(2, "Expunging message UID %d.\n", HEADER_DATA(h)->uid);
+      mutt_debug(2, "Expunging message UID %u.\n", HEADER_DATA(h)->uid);
 
       h->active = false;
       idata->ctx->size -= h->content->length;
@@ -1669,7 +1669,7 @@ struct ImapStatus *imap_mboxcache_get(struct ImapData *idata, const char *mbox,
       }
       status->uidvalidity = *(unsigned int *) uidvalidity;
       status->uidnext = uidnext ? *(unsigned int *) uidnext : 0;
-      mutt_debug(3, "hcache uidvalidity %d, uidnext %d\n", status->uidvalidity,
+      mutt_debug(3, "hcache uidvalidity %u, uidnext %u\n", status->uidvalidity,
                  status->uidnext);
     }
     mutt_hcache_free(hc, &uidvalidity);
@@ -2143,7 +2143,8 @@ static int imap_open_mailbox(struct Context *ctx)
       mutt_debug(3, "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 (mutt_str_strncasecmp("OK [UIDNEXT", pc, 11) == 0)
@@ -2151,7 +2152,8 @@ static int imap_open_mailbox(struct Context *ctx)
       mutt_debug(3, "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

diff --git a/imap/message.c b/imap/message.c
index 8f01cf6a74edb72830c594b8445e6e3858c9ebaa..a5823ee4c81a9b22e8958f00f10c5f2b3602c44d 100644 (file)
--- a/imap/message.c
+++ b/imap/message.c
@@ -328,7 +328,8 @@ static int msg_parse_fetch(struct ImapHeader *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);
     }
@@ -359,7 +360,8 @@ static int msg_parse_fetch(struct ImapHeader *h, char *s)
       while (isdigit((unsigned char) *s))
         *ptmp++ = *s++;
       *ptmp = '\0';
-      h->content_length = atoi(tmp);
+      if (mutt_str_atol(tmp, &h->content_length) < 0)
+        return -1;
     }
     else if ((mutt_str_strncasecmp("BODY", s, 4) == 0) ||
              (mutt_str_strncasecmp("RFC822.HEADER", s, 13) == 0))
@@ -406,7 +408,8 @@ static int msg_fetch_header(struct Context *ctx, struct ImapHeader *h, char *buf
 
   /* 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);
@@ -1005,7 +1008,7 @@ int imap_fetch_message(struct Context *ctx, struct Message *msg, int msgno)
   char *pc = NULL;
   long bytes;
   struct Progress progressbar;
-  int uid;
+  unsigned int uid;
   int cacheno;
   struct ImapCache *cache = NULL;
   bool read;
@@ -1096,7 +1099,8 @@ int imap_fetch_message(struct Context *ctx, struct Message *msg, int msgno)
         if (mutt_str_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."));

diff --git a/muttlib.c b/muttlib.c
index 46f6ffcc20eaf1157e6788d3bcc9e1586a7ab9c0..2b867ad9909b40dc7f0db8d89605ad198bdf1198 100644 (file)
--- a/muttlib.c
+++ b/muttlib.c
@@ -1594,3 +1594,59 @@ int mutt_inbox_cmp(const char *a, const char *b)
 
   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/protos.h b/protos.h
index 860fbd4a5e825528316d8f2904afedf844e4546d..649a9fff33e594230901b72e70cb79a98ee7a391 100644 (file)
--- a/protos.h
+++ b/protos.h
@@ -363,4 +363,7 @@ bool message_is_visible(struct Context *ctx, int index);
 int mutt_addrlist_to_intl(struct Address *a, char **err);
 int mutt_addrlist_to_local(struct Address *a);
 
+int mutt_atoui(const char *str, unsigned int *dst);
+int mutt_atoul(const char *str, unsigned long *dst);
+
 #endif /* _MUTT_PROTOS_H */