Michal Marek forwarded the bug report

https://bugzilla.novell.com/show_bug.cgi?id=332917 about a HTTP redirect to
FTP that caused memory havoc. His work together with my efforts created two
fixes:

#1 - FTP::file was moved to struct ftp_conn, because is has to be dealt with
     at connection cleanup, at which time the struct HandleData could be
     used by another connection.
     Also, the unused char *urlpath member is removed from struct FTP.

#2 - provide a Curl_reset_reqproto() function that frees
     data->reqdata.proto.* on connection setup if needed (that is if the
     SessionHandle was used by a different connection).

9 files changed, 116 insertions, 62 deletions

diff --git a/CHANGES b/CHANGES
index 87d36ae4f..bc798bb1b 100644
--- a/CHANGES
+++ b/CHANGES
@@ -7,6 +7,23 @@
                                   Changelog
 
 Daniel S (22 October 2007)
+- Michal Marek forwarded the bug report
+  https://bugzilla.novell.com/show_bug.cgi?id=332917 about a HTTP redirect to
+  FTP that caused memory havoc. His work together with my efforts created two
+  fixes:
+
+  #1 - FTP::file was moved to struct ftp_conn, because is has to be dealt with
+       at connection cleanup, at which time the struct HandleData could be
+       used by another connection.
+       Also, the unused char *urlpath member is removed from struct FTP.
+ 
+  #2 - provide a Curl_reset_reqproto() function that frees
+       data->reqdata.proto.* on connection setup if needed (that is if the
+       SessionHandle was used by a different connection).
+
+  A long-term goal is of course to somehow get rid of how the reqdata struct
+  is used, as it is too error-prone.
+ 
 - Bug report #1815530 (http://curl.haxx.se/bug/view.cgi?id=1815530) points out
   that specifying a proxy with a trailing slash didn't work (unless it also
   contained a port number).
diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index 91a938262..cbc24b8ba 100644
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -40,6 +40,7 @@ This release includes the following bugfixes:
  o CURLOPT_POSTFIELDS could fail to send binary data
  o specifying a proxy with a trailing slash didn't work (unless it also
    contained a port number)
+ o redirect from HTTP to FTP memory problem
 
 This release includes the following known bugs:
 
diff --git a/lib/file.c b/lib/file.c
index 6d4ae9892..bce92bd2a 100644
--- a/lib/file.c
+++ b/lib/file.c
@@ -125,8 +125,8 @@ const struct Curl_handler Curl_handler_file = {
  */
 CURLcode Curl_file_connect(struct connectdata *conn)
 {
-  char *real_path = curl_easy_unescape(conn->data, conn->data->reqdata.path, 0,
-                                       NULL);
+  struct SessionHandle *data = conn->data;
+  char *real_path = curl_easy_unescape(data, data->reqdata.path, 0, NULL);
   struct FILEPROTO *file;
   int fd;
 #if defined(WIN32) || defined(MSDOS) || defined(__EMX__)
@@ -137,16 +137,18 @@ CURLcode Curl_file_connect(struct connectdata *conn)
   if(!real_path)
     return CURLE_OUT_OF_MEMORY;
 
-  file = (struct FILEPROTO *)calloc(sizeof(struct FILEPROTO), 1);
-  if(!file) {
-    free(real_path);
-    return CURLE_OUT_OF_MEMORY;
-  }
-
-  if (conn->data->reqdata.proto.file)
-    free(conn->data->reqdata.proto.file);
+  /* If there already is a protocol-specific struct allocated for this
+     sessionhandle, deal with it */
+  Curl_reset_reqproto(conn);
 
-  conn->data->reqdata.proto.file = file;
+  if (!data->reqdata.proto.file) {
+    file = (struct FILEPROTO *)calloc(sizeof(struct FILEPROTO), 1);
+    if(!file) {
+      free(real_path);
+      return CURLE_OUT_OF_MEMORY;
+    }
+    data->reqdata.proto.file = file;
+  }
 
 #if defined(WIN32) || defined(MSDOS) || defined(__EMX__)
   /* If the first character is a slash, and there's
@@ -186,8 +188,8 @@ CURLcode Curl_file_connect(struct connectdata *conn)
   file->freepath = real_path; /* free this when done */
 
   file->fd = fd;
-  if(!conn->data->set.upload && (fd == -1)) {
-    failf(conn->data, "Couldn't open file %s", conn->data->reqdata.path);
+  if(!data->set.upload && (fd == -1)) {
+    failf(data, "Couldn't open file %s", data->reqdata.path);
     Curl_file_done(conn, CURLE_FILE_COULDNT_READ_FILE, FALSE);
     return CURLE_FILE_COULDNT_READ_FILE;
   }
diff --git a/lib/ftp.c b/lib/ftp.c
index 105830a2c..d6626f600 100644
--- a/lib/ftp.c
+++ b/lib/ftp.c
@@ -90,6 +90,7 @@
 #include "parsedate.h" /* for the week day and month names */
 #include "sockaddr.h" /* required for Curl_sockaddr_storage */
 #include "multiif.h"
+#include "url.h"
 
 #if defined(HAVE_INET_NTOA_R) && !defined(HAVE_INET_NTOA_R_DECL)
 #include "inet_ntoa_r.h"
@@ -261,7 +262,6 @@ const struct Curl_handler Curl_handler_ftps_proxy = {
 static void freedirs(struct connectdata *conn)
 {
   struct ftp_conn *ftpc = &conn->proto.ftpc;
-  struct FTP *ftp = conn->data->reqdata.proto.ftp;
 
   int i;
   if(ftpc->dirs) {
@@ -274,9 +274,9 @@ static void freedirs(struct connectdata *conn)
     free(ftpc->dirs);
     ftpc->dirs = NULL;
   }
-  if(ftp->file) {
-    free(ftp->file);
-    ftp->file = NULL;
+  if(ftpc->file) {
+    free(ftpc->file);
+    ftpc->file = NULL;
   }
 }
 
@@ -1339,8 +1339,9 @@ static CURLcode ftp_state_post_size(struct connectdata *conn)
 {
   CURLcode result = CURLE_OK;
   struct FTP *ftp = conn->data->reqdata.proto.ftp;
+  struct ftp_conn *ftpc = &conn->proto.ftpc;
 
-  if((ftp->transfer != FTPTRANSFER_BODY) && ftp->file) {
+  if((ftp->transfer != FTPTRANSFER_BODY) && ftpc->file) {
     /* if a "head"-like request is being made (on a file) */
 
     /* Determine if server can respond to REST command and therefore
@@ -1359,12 +1360,13 @@ static CURLcode ftp_state_post_type(struct connectdata *conn)
 {
   CURLcode result = CURLE_OK;
   struct FTP *ftp = conn->data->reqdata.proto.ftp;
+  struct ftp_conn *ftpc = &conn->proto.ftpc;
 
-  if((ftp->transfer == FTPTRANSFER_INFO) && ftp->file) {
+  if((ftp->transfer == FTPTRANSFER_INFO) && ftpc->file) {
     /* if a "head"-like request is being made (on a file) */
 
-    /* we know ftp->file is a valid pointer to a file name */
-    NBFTPSENDF(conn, "SIZE %s", ftp->file);
+    /* we know ftpc->file is a valid pointer to a file name */
+    NBFTPSENDF(conn, "SIZE %s", ftpc->file);
 
     state(conn, FTP_SIZE);
   }
@@ -1466,11 +1468,12 @@ static CURLcode ftp_state_post_mdtm(struct connectdata *conn)
   CURLcode result = CURLE_OK;
   struct FTP *ftp = conn->data->reqdata.proto.ftp;
   struct SessionHandle *data = conn->data;
+  struct ftp_conn *ftpc = &conn->proto.ftpc;
 
   /* If we have selected NOBODY and HEADER, it means that we only want file
      information. Which in FTP can't be much more than the file size and
      date. */
-  if(conn->bits.no_body && ftp->file &&
+  if(conn->bits.no_body && ftpc->file &&
      ftp_need_type(conn, data->set.prefer_ascii)) {
     /* The SIZE command is _not_ RFC 959 specified, and therefor many servers
        may not support it! It is however the only way we have to get a file's
@@ -1496,15 +1499,15 @@ static CURLcode ftp_state_post_mdtm(struct connectdata *conn)
 static CURLcode ftp_state_post_cwd(struct connectdata *conn)
 {
   CURLcode result = CURLE_OK;
-  struct FTP *ftp = conn->data->reqdata.proto.ftp;
   struct SessionHandle *data = conn->data;
+  struct ftp_conn *ftpc = &conn->proto.ftpc;
 
   /* Requested time of file or time-depended transfer? */
-  if((data->set.get_filetime || data->set.timecondition) && ftp->file) {
+  if((data->set.get_filetime || data->set.timecondition) && ftpc->file) {
 
     /* we have requested to get the modified-time of the file, this is a white
        spot as the MDTM is not mentioned in RFC959 */
-    NBFTPSENDF(conn, "MDTM %s", ftp->file);
+    NBFTPSENDF(conn, "MDTM %s", ftpc->file);
 
     state(conn, FTP_MDTM);
   }
@@ -1522,6 +1525,7 @@ static CURLcode ftp_state_ul_setup(struct connectdata *conn,
   CURLcode result = CURLE_OK;
   struct FTP *ftp = conn->data->reqdata.proto.ftp;
   struct SessionHandle *data = conn->data;
+  struct ftp_conn *ftpc = &conn->proto.ftpc;
   curl_off_t passed=0;
 
   if((data->reqdata.resume_from && !sizechecked) ||
@@ -1541,7 +1545,7 @@ static CURLcode ftp_state_ul_setup(struct connectdata *conn,
 
     if(data->reqdata.resume_from < 0 ) {
       /* Got no given size to start from, figure it out */
-      NBFTPSENDF(conn, "SIZE %s", ftp->file);
+      NBFTPSENDF(conn, "SIZE %s", ftpc->file);
       state(conn, FTP_STOR_SIZE);
       return result;
     }
@@ -1596,7 +1600,7 @@ static CURLcode ftp_state_ul_setup(struct connectdata *conn,
   } /* resume_from */
 
   NBFTPSENDF(conn, data->set.ftp_append?"APPE %s":"STOR %s",
-             ftp->file);
+             ftpc->file);
 
   state(conn, FTP_STOR);
 
@@ -1659,7 +1663,7 @@ static CURLcode ftp_state_quote(struct connectdata *conn,
       if (ftp->transfer != FTPTRANSFER_BODY)
         state(conn, FTP_STOP);
       else {
-        NBFTPSENDF(conn, "SIZE %s", ftp->file);
+        NBFTPSENDF(conn, "SIZE %s", ftpc->file);
         state(conn, FTP_RETR_SIZE);
       }
       break;
@@ -1961,6 +1965,7 @@ static CURLcode ftp_state_mdtm_resp(struct connectdata *conn,
   CURLcode result = CURLE_OK;
   struct SessionHandle *data=conn->data;
   struct FTP *ftp = data->reqdata.proto.ftp;
+  struct ftp_conn *ftpc = &conn->proto.ftpc;
 
   switch(ftpcode) {
   case 213:
@@ -1986,7 +1991,7 @@ static CURLcode ftp_state_mdtm_resp(struct connectdata *conn,
          we "emulate" a HTTP-style header in our output. */
 
       if(conn->bits.no_body &&
-         ftp->file &&
+         ftpc->file &&
          data->set.get_filetime &&
          (data->info.filetime>=0) ) {
         struct tm *tm;
@@ -2092,6 +2097,7 @@ static CURLcode ftp_state_post_retr_size(struct connectdata *conn,
   CURLcode result = CURLE_OK;
   struct SessionHandle *data=conn->data;
   struct FTP *ftp = data->reqdata.proto.ftp;
+  struct ftp_conn *ftpc = &conn->proto.ftpc;
 
   if (data->set.max_filesize && (filesize > data->set.max_filesize)) {
     failf(data, "Maximum file size exceeded");
@@ -2160,7 +2166,7 @@ static CURLcode ftp_state_post_retr_size(struct connectdata *conn,
   }
   else {
     /* no resume */
-    NBFTPSENDF(conn, "RETR %s", ftp->file);
+    NBFTPSENDF(conn, "RETR %s", ftpc->file);
     state(conn, FTP_RETR);
   }
 
@@ -2209,7 +2215,7 @@ static CURLcode ftp_state_rest_resp(struct connectdata *conn,
                                     ftpstate instate)
 {
   CURLcode result = CURLE_OK;
-  struct FTP *ftp = conn->data->reqdata.proto.ftp;
+  struct ftp_conn *ftpc = &conn->proto.ftpc;
 
   switch(instate) {
   case FTP_REST:
@@ -2231,7 +2237,7 @@ static CURLcode ftp_state_rest_resp(struct connectdata *conn,
       result = CURLE_FTP_COULDNT_USE_REST;
     }
     else {
-      NBFTPSENDF(conn, "RETR %s", ftp->file);
+      NBFTPSENDF(conn, "RETR %s", ftpc->file);
       state(conn, FTP_RETR);
     }
     break;
@@ -3047,11 +3053,9 @@ static CURLcode Curl_ftp_connect(struct connectdata *conn,
 
   *done = FALSE; /* default to not done yet */
 
-  if (data->reqdata.proto.ftp) {
-    Curl_ftp_disconnect(conn);
-    free(data->reqdata.proto.ftp);
-    data->reqdata.proto.ftp = NULL;
-  }
+  /* If there already is a protocol-specific struct allocated for this
+     sessionhandle, deal with it */
+  Curl_reset_reqproto(conn);
 
   result = ftp_init(conn);
   if(result)
@@ -3183,7 +3187,7 @@ static CURLcode Curl_ftp_done(struct connectdata *conn, CURLcode status,
     ftpc->prevpath = NULL; /* no path */
 
   } else {
-    size_t flen = ftp->file?strlen(ftp->file):0; /* file is "raw" already */
+    size_t flen = ftpc->file?strlen(ftpc->file):0; /* file is "raw" already */
     size_t dlen = strlen(path)-flen;
     if(!ftpc->cwdfail) {
       if(dlen && (data->set.ftp_filemethod != FTPFILE_NOCWD)) {
@@ -3476,6 +3480,7 @@ static CURLcode ftp_range(struct connectdata *conn)
 static CURLcode Curl_ftp_nextconnect(struct connectdata *conn)
 {
   struct SessionHandle *data=conn->data;
+  struct ftp_conn *ftpc = &conn->proto.ftpc;
   CURLcode result = CURLE_OK;
 
   /* the ftp struct is inited in Curl_ftp_connect() */
@@ -3499,7 +3504,7 @@ static CURLcode Curl_ftp_nextconnect(struct connectdata *conn)
       result = ftp_range(conn);
       if(result)
         ;
-      else if(data->set.ftp_list_only || !ftp->file) {
+      else if(data->set.ftp_list_only || !ftpc->file) {
         /* The specified path ends with a slash, and therefore we think this
            is a directory that is requested, use LIST. But before that we
            need to set ASCII transfer mode. */
@@ -3602,6 +3607,7 @@ static CURLcode Curl_ftp(struct connectdata *conn, bool *done)
     make sure we have a good 'struct FTP' to play with. For new connections,
     the struct FTP is allocated and setup in the Curl_ftp_connect() function.
   */
+  Curl_reset_reqproto(conn);
   retcode = ftp_init(conn);
   if(retcode)
     return retcode;
@@ -3800,12 +3806,16 @@ static CURLcode Curl_ftp_disconnect(struct connectdata *conn)
   */
 
   /* The FTP session may or may not have been allocated/setup at this point! */
+  /* FIXME: checking for conn->data->reqdata.proto.ftp is not correct here,
+   * the reqdata structure could be used by another connection already */
   if(conn->data->reqdata.proto.ftp) {
     (void)ftp_quit(conn); /* ignore errors on the QUIT */
 
     if(ftpc->entrypath) {
       struct SessionHandle *data = conn->data;
-      data->state.most_recent_ftp_entrypath = NULL;
+      if (data->state.most_recent_ftp_entrypath == ftpc->entrypath) {
+        data->state.most_recent_ftp_entrypath = NULL;
+      }
       free(ftpc->entrypath);
       ftpc->entrypath = NULL;
     }
@@ -3861,11 +3871,11 @@ CURLcode ftp_parse_url_path(struct connectdata *conn)
     if(data->reqdata.path &&
        data->reqdata.path[0] &&
        (data->reqdata.path[strlen(data->reqdata.path) - 1] != '/') )
-      ftp->file = data->reqdata.path;  /* this is a full file path */
+      ftpc->file = data->reqdata.path;  /* this is a full file path */
     else
-      ftp->file = NULL;
+      ftpc->file = NULL;
       /*
-        ftp->file is not used anywhere other than for operations on a file.
+        ftpc->file is not used anywhere other than for operations on a file.
         In other words, never for directory operations.
         So we can safely set it to NULL here and use it as a
         argument in dir/file decisions.
@@ -3877,7 +3887,7 @@ CURLcode ftp_parse_url_path(struct connectdata *conn)
     if(!path_to_use[0]) {
       /* no dir, no file */
       ftpc->dirdepth = 0;
-      ftp->file = NULL;
+      ftpc->file = NULL;
       break;
     }
     slash_pos=strrchr(cur_pos, '/');
@@ -3894,10 +3904,10 @@ CURLcode ftp_parse_url_path(struct connectdata *conn)
         return CURLE_OUT_OF_MEMORY;
       }
       ftpc->dirdepth = 1; /* we consider it to be a single dir */
-      ftp->file = slash_pos ? slash_pos+1 : cur_pos; /* rest is file name */
+      ftpc->file = slash_pos ? slash_pos+1 : cur_pos; /* rest is file name */
     }
     else
-      ftp->file = cur_pos;  /* this is a file name only */
+      ftpc->file = cur_pos;  /* this is a file name only */
     break;
 
   default: /* allow pretty much anything */
@@ -3959,26 +3969,26 @@ CURLcode ftp_parse_url_path(struct connectdata *conn)
         }
       }
     }
-    ftp->file = cur_pos;  /* the rest is the file name */
+    ftpc->file = cur_pos;  /* the rest is the file name */
   }
 
-  if(ftp->file && *ftp->file) {
-    ftp->file = curl_easy_unescape(conn->data, ftp->file, 0, NULL);
-    if(NULL == ftp->file) {
+  if(ftpc->file && *ftpc->file) {
+    ftpc->file = curl_easy_unescape(conn->data, ftpc->file, 0, NULL);
+    if(NULL == ftpc->file) {
       freedirs(conn);
       failf(data, "no memory");
       return CURLE_OUT_OF_MEMORY;
     }
-    if (isBadFtpString(ftp->file)) {
+    if (isBadFtpString(ftpc->file)) {
       freedirs(conn);
       return CURLE_URL_MALFORMAT;
     }
   }
   else
-    ftp->file=NULL; /* instead of point to a zero byte, we make it a NULL
+    ftpc->file=NULL; /* instead of point to a zero byte, we make it a NULL
                        pointer */
 
-  if(data->set.upload && !ftp->file && (ftp->transfer == FTPTRANSFER_BODY)) {
+  if(data->set.upload && !ftpc->file && (ftp->transfer == FTPTRANSFER_BODY)) {
     /* We need a file name when uploading. Return error! */
     failf(data, "Uploading to a URL without a file name!");
     return CURLE_URL_MALFORMAT;
@@ -3995,7 +4005,7 @@ CURLcode ftp_parse_url_path(struct connectdata *conn)
       return CURLE_OUT_OF_MEMORY;
     }
 
-    dlen = strlen(path) - (ftp->file?strlen(ftp->file):0);
+    dlen = strlen(path) - (ftpc->file?strlen(ftpc->file):0);
     if((dlen == strlen(ftpc->prevpath)) &&
        curl_strnequal(path, ftpc->prevpath, dlen)) {
       infof(data, "Request has same path as previous transfer\n");
diff --git a/lib/http.c b/lib/http.c
index b32ca1d5b..7c63097c9 100644
--- a/lib/http.c
+++ b/lib/http.c
@@ -1894,13 +1894,16 @@ CURLcode Curl_http(struct connectdata *conn, bool *done)
      the rest of the request in the PERFORM phase. */
   *done = TRUE;
 
+  /* If there already is a protocol-specific struct allocated for this
+     sessionhandle, deal with it */
+  Curl_reset_reqproto(conn);
+
   if(!data->reqdata.proto.http) {
     /* Only allocate this struct if we don't already have it! */
 
-    http = (struct HTTP *)malloc(sizeof(struct HTTP));
+    http = (struct HTTP *)calloc(sizeof(struct HTTP), 1);
     if(!http)
       return CURLE_OUT_OF_MEMORY;
-    memset(http, 0, sizeof(struct HTTP));
     data->reqdata.proto.http = http;
   }
   else
diff --git a/lib/ssh.c b/lib/ssh.c
index 25500a101..2587516f0 100644
--- a/lib/ssh.c
+++ b/lib/ssh.c
@@ -1846,10 +1846,9 @@ static CURLcode Curl_ssh_connect(struct connectdata *conn, bool *done)
   CURLcode result;
   struct SessionHandle *data = conn->data;
 
-  if (data->reqdata.proto.ssh) {
-    Curl_safefree(data->reqdata.proto.ssh);
-    data->reqdata.proto.ssh = NULL;
-  }
+  /* If there already is a protocol-specific struct allocated for this
+     sessionhandle, deal with it */
+  Curl_reset_reqproto(conn);
 
   result = ssh_init(conn);
   if (result)
diff --git a/lib/url.c b/lib/url.c
index 212e184f7..1fc11f1c7 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -2058,6 +2058,9 @@ static void conn_free(struct connectdata *conn)
   if(CURL_SOCKET_BAD != conn->sock[FIRSTSOCKET])
     sclose(conn->sock[FIRSTSOCKET]);
 
+  if (conn->data->reqdata.current_conn == conn) {
+    conn->data->reqdata.current_conn = NULL;
+  }
   Curl_safefree(conn->user);
   Curl_safefree(conn->passwd);
   Curl_safefree(conn->proxyuser);
@@ -4514,3 +4517,16 @@ CURLcode Curl_do_more(struct connectdata *conn)
 
   return result;
 }
+
+/* Called on connect, and if there's already a protocol-specific struct
+   allocated for a different connection, this frees it that it can be setup
+   properly later on. */
+void Curl_reset_reqproto(struct connectdata *conn)
+{
+  struct SessionHandle *data = conn->data;
+  if (data->reqdata.proto.generic && data->reqdata.current_conn != conn) {
+    free(data->reqdata.proto.generic);
+    data->reqdata.proto.generic = NULL;
+  }
+  data->reqdata.current_conn = conn;
+}
diff --git a/lib/url.h b/lib/url.h
index d7a75e0a5..e4f72eb7a 100644
--- a/lib/url.h
+++ b/lib/url.h
@@ -84,4 +84,9 @@ CURLcode Curl_doing_fdset(struct connectdata *conn,
                           int *max_fdp);
 #endif
 
+/* Called on connect, and if there's already a protocol-specific struct
+   allocated for a different connection, this frees it that it can be setup
+   properly later on. */
+void Curl_reset_reqproto(struct connectdata *conn);
+
 #endif
diff --git a/lib/urldata.h b/lib/urldata.h
index 8ee1d1989..b13ba42f1 100644
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -376,8 +376,6 @@ struct FTP {
   curl_off_t *bytecountp;
   char *user;    /* user name string */
   char *passwd;  /* password string */
-  char *urlpath; /* the originally given path part of the URL */
-  char *file;    /* decoded file */
 
   /* transfer a file/body or not, done as a typedefed enum just to make
      debuggers display the full symbol and not just the numerical value */
@@ -392,6 +390,7 @@ struct ftp_conn {
   char **dirs;   /* realloc()ed array for path components */
   int dirdepth;  /* number of entries used in the 'dirs' array */
   int diralloc;  /* number of entries allocated for the 'dirs' array */
+  char *file;    /* decoded file */
   char *cache;       /* data cache between getresponse()-calls */
   curl_off_t cache_size; /* size of cache in bytes */
   bool dont_check;  /* Set to TRUE to prevent the final (post-transfer)
@@ -807,6 +806,8 @@ struct HandleData {
     void *generic;
     struct SSHPROTO *ssh;
   } proto;
+  /* current user of this HandleData instance, or NULL */
+  struct connectdata *current_conn;
 };
 
 /*