strtoofft: reduce integer overflow risks globally

... make sure we bail out on overflows.

Reported-by: Brian Carpenter
Closes #1758

11 files changed, 180 insertions, 126 deletions

diff --git a/lib/cookie.c b/lib/cookie.c
index 6b678aeb8..eba2536ef 100644
--- a/lib/cookie.c
+++ b/lib/cookie.c
@@ -593,14 +593,20 @@ Curl_cookie_add(struct Curl_easy *data,
     } while(semiptr);
 
     if(co->maxage) {
-      co->expires =
-        curlx_strtoofft((*co->maxage=='\"')?
-                        &co->maxage[1]:&co->maxage[0], NULL, 10);
-      if(CURL_OFF_T_MAX - now < co->expires)
-        /* avoid overflow */
+      CURLofft offt;
+      offt = curlx_strtoofft((*co->maxage=='\"')?
+                             &co->maxage[1]:&co->maxage[0], NULL, 10,
+                             &co->expires);
+      if(offt == CURL_OFFT_FLOW)
+        /* overflow, used max value */
         co->expires = CURL_OFF_T_MAX;
-      else
-        co->expires += now;
+      else if(!offt) {
+        if(CURL_OFF_T_MAX - now < co->expires)
+          /* would overflow */
+          co->expires = CURL_OFF_T_MAX;
+        else
+          co->expires += now;
+      }
     }
     else if(co->expirestr) {
       /* Note that if the date couldn't get parsed for whatever reason,
@@ -753,7 +759,8 @@ Curl_cookie_add(struct Curl_easy *data,
         co->secure = strcasecompare(ptr, "TRUE")?TRUE:FALSE;
         break;
       case 4:
-        co->expires = curlx_strtoofft(ptr, NULL, 10);
+        if(curlx_strtoofft(ptr, NULL, 10, &co->expires))
+          badcookie = TRUE;
         break;
       case 5:
         co->name = strdup(ptr);
diff --git a/lib/file.c b/lib/file.c
index 666cbe75b..a8f29f2ca 100644
--- a/lib/file.c
+++ b/lib/file.c
@@ -139,26 +139,28 @@ static CURLcode file_range(struct connectdata *conn)
   struct Curl_easy *data = conn->data;
 
   if(data->state.use_range && data->state.range) {
-    from=curlx_strtoofft(data->state.range, &ptr, 0);
+    CURLofft from_t;
+    CURLofft to_t;
+    from_t = curlx_strtoofft(data->state.range, &ptr, 0, &from);
+    if(from_t == CURL_OFFT_FLOW)
+      return CURLE_RANGE_ERROR;
     while(*ptr && (ISSPACE(*ptr) || (*ptr=='-')))
       ptr++;
-    to=curlx_strtoofft(ptr, &ptr2, 0);
-    if(ptr == ptr2) {
-      /* we didn't get any digit */
-      to=-1;
-    }
-    if((-1 == to) && (from>=0)) {
+    to_t = curlx_strtoofft(ptr, &ptr2, 0, &to);
+    if(to_t == CURL_OFFT_FLOW)
+      return CURLE_RANGE_ERROR;
+    if((to_t == CURL_OFFT_INVAL) && !from_t) {
       /* X - */
       data->state.resume_from = from;
       DEBUGF(infof(data, "RANGE %" CURL_FORMAT_CURL_OFF_T " to end of file\n",
                    from));
     }
-    else if(from < 0) {
+    else if((from_t == CURL_OFFT_INVAL) && !to_t) {
       /* -Y */
-      data->req.maxdownload = -from;
-      data->state.resume_from = from;
+      data->req.maxdownload = to;
+      data->state.resume_from = -to;
       DEBUGF(infof(data, "RANGE the last %" CURL_FORMAT_CURL_OFF_T " bytes\n",
-                   -from));
+                   to));
     }
     else {
       /* X-Y */
diff --git a/lib/ftp.c b/lib/ftp.c
index 6e86e5386..481b14a88 100644
--- a/lib/ftp.c
+++ b/lib/ftp.c
@@ -2260,11 +2260,13 @@ static CURLcode ftp_state_size_resp(struct connectdata *conn,
 {
   CURLcode result = CURLE_OK;
   struct Curl_easy *data=conn->data;
-  curl_off_t filesize;
+  curl_off_t filesize = -1;
   char *buf = data->state.buffer;
 
   /* get the size from the ascii string: */
-  filesize = (ftpcode == 213)?curlx_strtoofft(buf+4, NULL, 0):-1;
+  if(ftpcode == 213)
+    /* ignores parsing errors, which will make the size remain unknown */
+    (void)curlx_strtoofft(buf+4, NULL, 0, &filesize);
 
   if(instate == FTP_SIZE) {
 #ifdef CURL_FTP_HTTPSTYLE_HEAD
@@ -2435,7 +2437,7 @@ static CURLcode ftp_state_get_resp(struct connectdata *conn,
         /* if we have nothing but digits: */
         if(bytes++) {
           /* get the number! */
-          size = curlx_strtoofft(bytes, NULL, 0);
+          (void)curlx_strtoofft(bytes, NULL, 0, &size);
         }
       }
     }
@@ -3466,31 +3468,32 @@ static CURLcode ftp_range(struct connectdata *conn)
 {
   curl_off_t from, to;
   char *ptr;
-  char *ptr2;
   struct Curl_easy *data = conn->data;
   struct ftp_conn *ftpc = &conn->proto.ftpc;
 
   if(data->state.use_range && data->state.range) {
-    from=curlx_strtoofft(data->state.range, &ptr, 0);
+    CURLofft from_t;
+    CURLofft to_t;
+    from_t = curlx_strtoofft(data->state.range, &ptr, 0, &from);
+    if(from_t == CURL_OFFT_FLOW)
+      return CURLE_RANGE_ERROR;
     while(*ptr && (ISSPACE(*ptr) || (*ptr=='-')))
       ptr++;
-    to=curlx_strtoofft(ptr, &ptr2, 0);
-    if(ptr == ptr2) {
-      /* we didn't get any digit */
-      to=-1;
-    }
-    if((-1 == to) && (from>=0)) {
+    to_t = curlx_strtoofft(ptr, NULL, 0, &to);
+    if(to_t == CURL_OFFT_FLOW)
+      return CURLE_RANGE_ERROR;
+    if((to_t == CURL_OFFT_INVAL) && !from_t) {
       /* X - */
       data->state.resume_from = from;
       DEBUGF(infof(conn->data, "FTP RANGE %" CURL_FORMAT_CURL_OFF_T
                    " to end of file\n", from));
     }
-    else if(from < 0) {
+    else if(!to_t && (from_t == CURL_OFFT_INVAL)) {
       /* -Y */
-      data->req.maxdownload = -from;
-      data->state.resume_from = from;
+      data->req.maxdownload = to;
+      data->state.resume_from = -to;
       DEBUGF(infof(conn->data, "FTP RANGE the last %" CURL_FORMAT_CURL_OFF_T
-                   " bytes\n", -from));
+                   " bytes\n", to));
     }
     else {
       /* X-Y */
diff --git a/lib/ftplistparser.c b/lib/ftplistparser.c
index 2acce31d8..47fe1733b 100644
--- a/lib/ftplistparser.c
+++ b/lib/ftplistparser.c
@@ -609,16 +609,18 @@ size_t Curl_ftp_parselist(char *buffer, size_t size, size_t nmemb,
             char *p;
             curl_off_t fsize;
             finfo->b_data[parser->item_offset + parser->item_length - 1] = 0;
-            fsize = curlx_strtoofft(finfo->b_data+parser->item_offset, &p, 10);
-            if(p[0] == '\0' && fsize != CURL_OFF_T_MAX &&
-                               fsize != CURL_OFF_T_MIN) {
-              parser->file_data->info.flags |= CURLFINFOFLAG_KNOWN_SIZE;
-              parser->file_data->info.size = fsize;
+            if(!curlx_strtoofft(finfo->b_data+parser->item_offset,
+                                &p, 10, &fsize)) {
+              if(p[0] == '\0' && fsize != CURL_OFF_T_MAX &&
+                 fsize != CURL_OFF_T_MIN) {
+                parser->file_data->info.flags |= CURLFINFOFLAG_KNOWN_SIZE;
+                parser->file_data->info.size = fsize;
+              }
+              parser->item_length = 0;
+              parser->item_offset = 0;
+              parser->state.UNIX.main = PL_UNIX_TIME;
+              parser->state.UNIX.sub.time = PL_UNIX_TIME_PREPART1;
             }
-            parser->item_length = 0;
-            parser->item_offset = 0;
-            parser->state.UNIX.main = PL_UNIX_TIME;
-            parser->state.UNIX.sub.time = PL_UNIX_TIME_PREPART1;
           }
           else if(!ISDIGIT(c)) {
             PL_ERROR(conn, CURLE_FTP_BAD_FILE_LIST);
@@ -935,19 +937,9 @@ size_t Curl_ftp_parselist(char *buffer, size_t size, size_t nmemb,
             }
             else {
               char *endptr;
-              finfo->size = curlx_strtoofft(finfo->b_data +
-                                            parser->item_offset,
-                                            &endptr, 10);
-              if(!*endptr) {
-                if(finfo->size == CURL_OFF_T_MAX ||
-                   finfo->size == CURL_OFF_T_MIN) {
-                  if(errno == ERANGE) {
-                    PL_ERROR(conn, CURLE_FTP_BAD_FILE_LIST);
-                    return bufflen;
-                  }
-                }
-              }
-              else {
+              if(curlx_strtoofft(finfo->b_data +
+                                 parser->item_offset,
+                                 &endptr, 10, &finfo->size)) {
                 PL_ERROR(conn, CURLE_FTP_BAD_FILE_LIST);
                 return bufflen;
               }
diff --git a/lib/http.c b/lib/http.c
index d66b8482f..35c7c3d43 100644
--- a/lib/http.c
+++ b/lib/http.c
@@ -3486,28 +3486,32 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data,
     /* Check for Content-Length: header lines to get size */
     if(!k->ignorecl && !data->set.ignorecl &&
        checkprefix("Content-Length:", k->p)) {
-      curl_off_t contentlength = curlx_strtoofft(k->p+15, NULL, 10);
-      if(data->set.max_filesize &&
-         contentlength > data->set.max_filesize) {
-        failf(data, "Maximum file size exceeded");
-        return CURLE_FILESIZE_EXCEEDED;
-      }
-      if(contentlength >= 0) {
-        k->size = contentlength;
-        k->maxdownload = k->size;
-        /* we set the progress download size already at this point
-           just to make it easier for apps/callbacks to extract this
-           info as soon as possible */
-        Curl_pgrsSetDownloadSize(data, k->size);
-      }
-      else {
-        /* Negative Content-Length is really odd, and we know it
-           happens for example when older Apache servers send large
-           files */
-        streamclose(conn, "negative content-length");
-        infof(data, "Negative content-length: %" CURL_FORMAT_CURL_OFF_T
-              ", closing after transfer\n", contentlength);
+      curl_off_t contentlength;
+      if(!curlx_strtoofft(k->p+15, NULL, 10, &contentlength)) {
+        if(data->set.max_filesize &&
+           contentlength > data->set.max_filesize) {
+          failf(data, "Maximum file size exceeded");
+          return CURLE_FILESIZE_EXCEEDED;
+        }
+        if(contentlength >= 0) {
+          k->size = contentlength;
+          k->maxdownload = k->size;
+          /* we set the progress download size already at this point
+             just to make it easier for apps/callbacks to extract this
+             info as soon as possible */
+          Curl_pgrsSetDownloadSize(data, k->size);
+        }
+        else {
+          /* Negative Content-Length is really odd, and we know it
+             happens for example when older Apache servers send large
+             files */
+          streamclose(conn, "negative content-length");
+          infof(data, "Negative content-length: %" CURL_FORMAT_CURL_OFF_T
+                ", closing after transfer\n", contentlength);
+        }
       }
+      else
+        infof(data, "Illegal Content-Length: header\n");
     }
     /* check for Content-Type: header lines to get the MIME-type */
     else if(checkprefix("Content-Type:", k->p)) {
@@ -3682,11 +3686,11 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data,
 
       /* if it truly stopped on a digit */
       if(ISDIGIT(*ptr)) {
-        k->offset = curlx_strtoofft(ptr, NULL, 10);
-
-        if(data->state.resume_from == k->offset)
-          /* we asked for a resume and we got it */
-          k->content_range = TRUE;
+        if(!curlx_strtoofft(ptr, NULL, 10, &k->offset)) {
+          if(data->state.resume_from == k->offset)
+            /* we asked for a resume and we got it */
+            k->content_range = TRUE;
+        }
       }
       else
         data->state.resume_from = 0; /* get everything */
diff --git a/lib/http_chunks.c b/lib/http_chunks.c
index 1bdf6974c..fce00c21e 100644
--- a/lib/http_chunks.c
+++ b/lib/http_chunks.c
@@ -5,7 +5,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2016, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2017, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -158,9 +158,7 @@ CHUNKcode Curl_httpchunk_read(struct connectdata *conn,
           return CHUNKE_ILLEGAL_HEX;
         }
 
-        ch->datasize=curlx_strtoofft(ch->hexbuffer, &endptr, 16);
-        if((ch->datasize == CURL_OFF_T_MAX) && (errno == ERANGE))
-          /* overflow is an error */
+        if(curlx_strtoofft(ch->hexbuffer, &endptr, 16, &ch->datasize))
           return CHUNKE_ILLEGAL_HEX;
         ch->state = CHUNK_LF; /* now wait for the CRLF */
       }
diff --git a/lib/http_proxy.c b/lib/http_proxy.c
index 36567d36f..5c5f8fb2c 100644
--- a/lib/http_proxy.c
+++ b/lib/http_proxy.c
@@ -523,8 +523,8 @@ static CURLcode CONNECT(struct connectdata *conn,
                   k->httpcode);
           }
           else {
-            s->cl = curlx_strtoofft(s->line_start +
-                                    strlen("Content-Length:"), NULL, 10);
+            (void)curlx_strtoofft(s->line_start +
+                                  strlen("Content-Length:"), NULL, 10, &s->cl);
           }
         }
         else if(Curl_compareheader(s->line_start, "Connection:", "close"))
diff --git a/lib/imap.c b/lib/imap.c
index 48af2902a..bc20110c2 100644
--- a/lib/imap.c
+++ b/lib/imap.c
@@ -1070,10 +1070,11 @@ static CURLcode imap_state_fetch_resp(struct connectdata *conn, int imapcode,
 
   if(*ptr == '{') {
     char *endptr;
-    size = curlx_strtoofft(ptr + 1, &endptr, 10);
-    if(endptr - ptr > 1 && endptr[0] == '}' &&
-       endptr[1] == '\r' && endptr[2] == '\0')
-      parsed = TRUE;
+    if(!curlx_strtoofft(ptr + 1, &endptr, 10, &size)) {
+      if(endptr - ptr > 1 && endptr[0] == '}' &&
+         endptr[1] == '\r' && endptr[2] == '\0')
+        parsed = TRUE;
+    }
   }
 
   if(parsed) {
diff --git a/lib/ssh.c b/lib/ssh.c
index 9443e24b9..cc5178fe7 100644
--- a/lib/ssh.c
+++ b/lib/ssh.c
@@ -2233,18 +2233,25 @@ static CURLcode ssh_statemach_act(struct connectdata *conn, bool *block)
           curl_off_t from, to;
           char *ptr;
           char *ptr2;
+          CURLofft to_t;
+          CURLofft from_t;
 
-          from=curlx_strtoofft(conn->data->state.range, &ptr, 0);
+          from_t = curlx_strtoofft(conn->data->state.range, &ptr, 0, &from);
+          if(from_t == CURL_OFFT_FLOW)
+            return CURLE_RANGE_ERROR;
           while(*ptr && (ISSPACE(*ptr) || (*ptr=='-')))
             ptr++;
-          to=curlx_strtoofft(ptr, &ptr2, 0);
-          if((ptr == ptr2) /* no "to" value given */
+          to_t = curlx_strtoofft(ptr, &ptr2, 0, &to);
+          if(to_t == CURL_OFFT_FLOW)
+            return CURLE_RANGE_ERROR;
+          if((to_t == CURL_OFFT_INVAL) /* no "to" value given */
              || (to >= size)) {
             to = size - 1;
           }
-          if(from < 0) {
+          if(from_t) {
             /* from is relative to end of file */
-            from += size;
+            from = size - to;
+            to = size - 1;
           }
           if(from > size) {
             failf(data, "Offset (%"
diff --git a/lib/strtoofft.c b/lib/strtoofft.c
index c2adc7280..f057bb1d6 100644
--- a/lib/strtoofft.c
+++ b/lib/strtoofft.c
@@ -5,7 +5,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2016, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2017, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -20,6 +20,7 @@
  *
  ***************************************************************************/
 
+#include <errno.h>
 #include "curl_setup.h"
 
 #include "strtoofft.h"
@@ -32,6 +33,30 @@
  * https://www.opengroup.org/onlinepubs/009695399/functions/strtoimax.html
  */
 
+#if (CURL_SIZEOF_CURL_OFF_T > CURL_SIZEOF_LONG)
+#  ifdef HAVE_STRTOLL
+#    define strtooff strtoll
+#  else
+#    if defined(_MSC_VER) && (_MSC_VER >= 1300) && (_INTEGRAL_MAX_BITS >= 64)
+#      if defined(_SAL_VERSION)
+         _Check_return_ _CRTIMP __int64 __cdecl _strtoi64(
+             _In_z_ const char *_String,
+             _Out_opt_ _Deref_post_z_ char **_EndPtr, _In_ int _Radix);
+#      else
+         _CRTIMP __int64 __cdecl _strtoi64(const char *_String,
+                                           char **_EndPtr, int _Radix);
+#      endif
+#      define strtooff _strtoi64
+#    else
+       curl_off_t curlx_strtoll(const char *nptr, char **endptr, int base);
+#      define strtooff curlx_strtoll
+#      define NEED_CURL_STRTOLL 1
+#    endif
+#  endif
+#else
+#  define strtooff strtol
+#endif
+
 #ifdef NEED_CURL_STRTOLL
 
 /* Range tests can be used for alphanum decoding if characters are consecutive,
@@ -48,11 +73,10 @@ static const char valchars[] =
 static int get_char(char c, int base);
 
 /**
- * Emulated version of the strtoll function.  This extracts a long long
+ * Custom version of the strtooff function.  This extracts a curl_off_t
  * value from the given input string and returns it.
  */
-curl_off_t
-curlx_strtoll(const char *nptr, char **endptr, int base)
+static curl_off_t strtooff(const char *nptr, char **endptr, int base)
 {
   char *end;
   int is_negative = 0;
@@ -186,3 +210,34 @@ static int get_char(char c, int base)
   return value;
 }
 #endif  /* Only present if we need strtoll, but don't have it. */
+
+/*
+ * Parse a *positive* up to 64 bit number written in ascii.
+ */
+CURLofft curlx_strtoofft(const char *str, char **endp, int base,
+                         curl_off_t *num)
+{
+  char *end;
+  curl_off_t number;
+  errno = 0;
+  *num = 0; /* clear by default */
+  while(str && *str && ISSPACE(*str))
+    str++;
+  if('-' == *str) {
+    if(endp)
+      *endp = (char *)str; /* didn't actually move */
+    return CURL_OFFT_INVAL; /* nothing parsed */
+  }
+  number = strtooff(str, &end, base);
+  if(endp)
+    *endp = end;
+  if(errno == ERANGE)
+    /* overflow/underflow */
+    return CURL_OFFT_FLOW;
+  else if(str == end)
+    /* nothing parsed */
+    return CURL_OFFT_INVAL;
+
+  *num = number;
+  return CURL_OFFT_OK;
+}
diff --git a/lib/strtoofft.h b/lib/strtoofft.h
index f4039f3a3..ac1fa4aa4 100644
--- a/lib/strtoofft.h
+++ b/lib/strtoofft.h
@@ -7,7 +7,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2014, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2017, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -40,30 +40,6 @@
  * of 'long' the conversion function to use is strtol().
  */
 
-#if (CURL_SIZEOF_CURL_OFF_T > CURL_SIZEOF_LONG)
-#  ifdef HAVE_STRTOLL
-#    define curlx_strtoofft strtoll
-#  else
-#    if defined(_MSC_VER) && (_MSC_VER >= 1300) && (_INTEGRAL_MAX_BITS >= 64)
-#      if defined(_SAL_VERSION)
-         _Check_return_ _CRTIMP __int64 __cdecl _strtoi64(
-             _In_z_ const char *_String,
-             _Out_opt_ _Deref_post_z_ char **_EndPtr, _In_ int _Radix);
-#      else
-         _CRTIMP __int64 __cdecl _strtoi64(const char *_String,
-                                           char **_EndPtr, int _Radix);
-#      endif
-#      define curlx_strtoofft _strtoi64
-#    else
-       curl_off_t curlx_strtoll(const char *nptr, char **endptr, int base);
-#      define curlx_strtoofft curlx_strtoll
-#      define NEED_CURL_STRTOLL 1
-#    endif
-#  endif
-#else
-#  define curlx_strtoofft strtol
-#endif
-
 #if (CURL_SIZEOF_CURL_OFF_T == 4)
 #  define CURL_OFF_T_MAX CURL_OFF_T_C(0x7FFFFFFF)
 #else
@@ -72,4 +48,13 @@
 #endif
 #define CURL_OFF_T_MIN (-CURL_OFF_T_MAX - CURL_OFF_T_C(1))
 
+typedef enum {
+  CURL_OFFT_OK,    /* parsed fine */
+  CURL_OFFT_FLOW,  /* over or underflow */
+  CURL_OFFT_INVAL  /* nothing was parsed */
+} CURLofft;
+
+CURLofft curlx_strtoofft(const char *str, char **endp, int base,
+                         curl_off_t *num);
+
 #endif /* HEADER_CURL_STRTOOFFT_H */