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 8529d9de70a9a99f37f30ba81c5428d518ffc77f..554aeb26d2b27964db3b55e57f4a2c5b766f737d 100644 (file)
--- a/imap/command.c
+++ b/imap/command.c
@@ -33,6 +33,7 @@
 
 #include <ctype.h>
 #include <stdlib.h>
+#include <errno.h>
 
 #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 b3e780c811ffc6aefe583e06384f9f62bf0731cb..8a089545e6c6bf9b1e5ffb9f95bc5e7c230e307c 100644 (file)
--- 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 4c8821659e7bced5a0db08a885b6cc11319e2270..4caa29e448bc335c286917a0c9545592e0a0ee62 100644 (file)
--- 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 224232b8257e3273aed8fd790ac6b2cf98ac4c8b..583d2fffae9ae7a40e750ed0c944ef55ea03e219 100644 (file)
--- 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 ee85cb90146b42e4c72b79be3e494995728cabc8..61ac700dee1f0002031c5bd76b9bd898975001a7 100644 (file)
--- 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 *);