diff options
| author | Daniel Stenberg <daniel@haxx.se> | 2018-10-02 09:58:04 +0200 |
|---|---|---|
| committer | Daniel Stenberg <daniel@haxx.se> | 2018-10-19 09:39:12 +0200 |
| commit | 30a2aea69434714b90c1725eccb6194b2b1331d9 (patch) | |
| tree | 8163e8e1853d2ea392874008500cbfebc0a829ba | |
| parent | e693a15722c7400cd945fd59fe0a4c52700c0552 (diff) | |
| download | curl-bagder/double-free-curl_follow.tar.gz | |
multi: avoid double-freebagder/double-free-curl_follow
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
| -rw-r--r-- | lib/multi.c | 13 | ||||
| -rw-r--r-- | lib/transfer.c | 3 |
2 files changed, 6 insertions, 10 deletions
diff --git a/lib/multi.c b/lib/multi.c index acf9ecc6e..62df95dd1 100644 --- a/lib/multi.c +++ b/lib/multi.c @@ -1705,7 +1705,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) { @@ -1713,19 +1712,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; @@ -1985,16 +1981,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 */ @@ -2006,6 +2000,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); diff --git a/lib/transfer.c b/lib/transfer.c index 2a348b687..deb0ec786 100644 --- a/lib/transfer.c +++ b/lib/transfer.c @@ -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; |