Eygene Ryabinkin fixed a use-after-free issue with HTTP transfers with the

multi interface

3 files changed, 26 insertions, 1 deletions

diff --git a/CHANGES b/CHANGES
index 129dbdc34..7c473de95 100644
--- a/CHANGES
+++ b/CHANGES
@@ -7,6 +7,26 @@
                                   Changelog
 
 Daniel (10 March 2007)
+- Eygene Ryabinkin:
+
+  The problem is the following: when we're calling Curl_done and it decides to
+  keep the connection opened ('left intact'), then the caller is not notified
+  that the connection was done via the NULLifying of the pointer, so some easy
+  handle is keeping the pointer to this connection.
+
+  Later ConnectionExists can select such connection for reuse even if we're
+  not pipelining: pipeLen is zero, so the (pipeLen > 0 && !canPipeline) is
+  false and we can reuse this connection for another easy handle. But thus the
+  connection will be shared between two easy handles if the handle that wants
+  to take the ownership is not the same as was not notified of the connection
+  was done in Curl_done. And when some of these easy handles will get their
+  connection really freed the another one will still keep the pointer.
+
+  My fix was rather trivial: I just added the NULLification to the 'else'
+  branch in the Curl_done. My tests with Git and ElectricFence showed no
+  problems both for HTTP pulling and cloning. Repository size is about 250 Mb,
+  so it was a considerable amount of Curl's work.
+
 - Bryan Henderson introduces two things:
   1) the progress callback gets called more frequently (at times)
   2) libcurl *might* call the callback when it receives a signal:
diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index 469a18cde..2d5030026 100644
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -41,6 +41,7 @@ This release includes the following bugfixes:
  o CURLOPT_INTERFACE for ipv6
  o the progress callback can get called more frequently
  o libcurl might call the progress callback when it receives a signal
+ o use-after-free issue with HTTP transfers with the multi interface
 
 This release includes the following known bugs:
 
@@ -62,6 +63,6 @@ advice from friends like these:
  Rob Crittenden, Robert A. Monat, Dan Fandrich, Duncan Mac-Vicar Prett,
  Michal Marek, Robson Braga Araujo, Ian Turner, Linus Nielsen Feltzing,
  Ravi Pratap, Adam D. Moss, Jose Kahan, Hang Kin Lau, Justin Fletcher,
- Robert Iakobashvili, Bryan Henderson
+ Robert Iakobashvili, Bryan Henderson, Eygene Ryabinkin
 
         Thanks! (and sorry if I forgot to mention someone)
diff --git a/lib/url.c b/lib/url.c
index b80d79239..644cec39a 100644
--- a/lib/url.c
+++ b/lib/url.c
@@ -4240,6 +4240,10 @@ CURLcode Curl_done(struct connectdata **connp,
     infof(data, "Connection #%ld to host %s left intact\n",
           conn->connectindex,
           conn->bits.httpproxy?conn->proxy.dispname:conn->host.dispname);
+
+    *connp = NULL; /* to make the caller of this function better detect that
+                      this connection is handed over and no longer used from
+                      this point on */
   }
 
   return result;