git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/2] http: clean up leak in init_curl_http_auth

From
Jeff King <peff@peff.net>
Date
Apr 13, 2012, 06:18 UTC
Message-ID
<20120413061835.GA13690@sigill.intra.peff.net>
In-Reply-To
<20120413061622.GA27591@sigill.intra.peff.net>

When we have a credential to give to curl, we must copy it into a "user:pass" buffer and then hand the buffer to curl. Old versions of curl did not copy the buffer, and we were expected to keep it valid. Newer versions of curl will copy the buffer.

Our solution was to use a strbuf and detach it, giving ownership of the resulting buffer to curl. However, this meant that we were leaking the buffer on newer versions of curl, since curl was just copying it and throwing away the string we passed. Furthermore, when we replaced a credential (e.g., because our original one was rejected), we were also leaking on both old and new versions of curl.

This got even worse in the last patch, which started replacing the credential (and thus leaking) on every http request.

Instead, let's use a static buffer to make the ownership more clear and less leaky. We already keep a static "struct credential", so we are only handling a single credential at a time, anyway.

Signed-off-by: Jeff King <peff@peff.net>
---
The "old" version was prior to 7.17.0, from 2007. That is probably still
new enough to care about. But at some point we should probably make a
sweep through http.c and get rid of conditional blocks for really old
cruft.
 http.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/http.c b/http.c
index c6dc9b7..eaf7f40 100644
--- a/http.c
+++ b/http.c
@@ -211,12 +211,12 @@ static int http_options(const char *var, const char *value, void *cb)
 static void init_curl_http_auth(CURL *result)
 {
 	if (http_auth.username) {
-		struct strbuf up = STRBUF_INIT;
+		static struct strbuf up = STRBUF_INIT;
 		credential_fill(&http_auth);
+		strbuf_reset(&up);
 		strbuf_addf(&up, "%s:%s",
 			    http_auth.username, http_auth.password);
-		curl_easy_setopt(result, CURLOPT_USERPWD,
-				 strbuf_detach(&up, NULL));
+		curl_easy_setopt(result, CURLOPT_USERPWD, up.buf);
 	}
 }
 
-- 
1.7.9.6.6.g6b3b56
Previous: Jeff KingNext: Jeff King
Message 11 of 12 in “Cannot clone the git repository shared over http with authorization.”
  1. Artur R. CzechowskiApr 1, 2012
  2. Clemens BuchacherApr 1, 2012
  3. Clemens BuchacherApr 1, 2012
  4. Jeff KingApr 2, 2012
  5. Clemens BuchacherApr 10, 2012
  6. 1/2 http auth fails with multiple curl handlesClemens Buchacher, Apr 10, 2012
  7. 2/2 fix http auth with multiple curl handlesClemens Buchacher, Apr 10, 2012
  8. Jeff KingApr 12, 2012
  9. Junio C HamanoApr 12, 2012
  10. Jeff KingApr 13, 2012
  11. 1/2 http: clean up leak in init_curl_http_authJeff King, Apr 13, 2012
  12. 2/2 http: use newer curl options for setting credentialsJeff King, Apr 13, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.