]> granicus.if.org Git - curl/commitdiff
multi: avoid double-free
authorDaniel Stenberg <daniel@haxx.se>
Tue, 2 Oct 2018 07:58:04 +0000 (09:58 +0200)
committerDaniel Stenberg <daniel@haxx.se>
Fri, 19 Oct 2018 13:29:31 +0000 (15:29 +0200)
Curl_follow() no longer frees the string. Make sure it happens in the
caller function, like we normally handle allocations.

This bug was introduced with the use of the URL API internally, it has
never been in a release version

Reported-by: Dario Weißer
Closes #3149

lib/multi.c
lib/transfer.c

index 9485f7857f1d279da67f9020d1f33b03bb613e31..7c691a1b84de29f6ab5eeab576a92e684bcba0a6 100644 (file)
@@ -1707,7 +1707,6 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
           char *newurl = NULL;
           followtype follow = FOLLOW_NONE;
           CURLcode drc;
-          bool retry = FALSE;
 
           drc = Curl_retry_request(data->easy_conn, &newurl);
           if(drc) {
@@ -1715,19 +1714,16 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
             result = drc;
             stream_error = TRUE;
           }
-          else
-            retry = (newurl)?TRUE:FALSE;
 
           Curl_posttransfer(data);
           drc = multi_done(&data->easy_conn, result, FALSE);
 
           /* When set to retry the connection, we must to go back to
            * the CONNECT state */
-          if(retry) {
+          if(newurl) {
             if(!drc || (drc == CURLE_SEND_ERROR)) {
               follow = FOLLOW_RETRY;
               drc = Curl_follow(data, newurl, follow);
-              newurl = NULL; /* freed by Curl_follow() */
               if(!drc) {
                 multistate(data, CURLM_STATE_CONNECT);
                 rc = CURLM_CALL_MULTI_PERFORM;
@@ -1987,16 +1983,14 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
           else
             follow = FOLLOW_RETRY;
           result = multi_done(&data->easy_conn, CURLE_OK, FALSE);
-          if(result)
-            /* Curl_follow() would otherwise free this */
-            free(newurl);
-          else {
+          if(!result) {
             result = Curl_follow(data, newurl, follow);
             if(!result) {
               multistate(data, CURLM_STATE_CONNECT);
               rc = CURLM_CALL_MULTI_PERFORM;
             }
           }
+          free(newurl);
         }
         else {
           /* after the transfer is done, go DONE */
@@ -2008,6 +2002,7 @@ static CURLMcode multi_runsingle(struct Curl_multi *multi,
             newurl = data->req.location;
             data->req.location = NULL;
             result = Curl_follow(data, newurl, FOLLOW_FAKE);
+            free(newurl);
             if(result) {
               stream_error = TRUE;
               result = multi_done(&data->easy_conn, result, TRUE);
index 2a348b68742050f7716b57c043dae007870883c2..deb0ec7864bb198fea5e3bf3bfe82be3ac791ed0 100644 (file)
@@ -1458,6 +1458,8 @@ CURLcode Curl_posttransfer(struct Curl_easy *data)
 /*
  * Curl_follow() handles the URL redirect magic. Pass in the 'newurl' string
  * as given by the remote server and set up the new URL to request.
+ *
+ * This function DOES NOT FREE the given url.
  */
 CURLcode Curl_follow(struct Curl_easy *data,
                      char *newurl,    /* the Location: string */
@@ -1515,7 +1517,6 @@ CURLcode Curl_follow(struct Curl_easy *data,
 
   DEBUGASSERT(data->state.uh);
   uc = curl_url_set(data->state.uh, CURLUPART_URL, newurl, 0);
-  free(newurl);
   if(uc)
     /* TODO: consider an error code remap here */
     return CURLE_URL_MALFORMAT;