http-walker: store url in a strbuf
authorJeff King <peff@peff.net>
Thu, 24 Sep 2015 21:07:31 +0000 (17:07 -0400)
committerJunio C Hamano <gitster@pobox.com>
Fri, 25 Sep 2015 17:18:18 +0000 (10:18 -0700)
We do an unchecked sprintf directly into our url buffer.
This doesn't overflow because we know that it was sized for
"$base/objects/info/http-alternates", and we are writing
"$base/objects/info/alternates", which must be smaller. But
that is not immediately obvious to a reader who is looking
for buffer overflows. Let's switch to a strbuf, so that we
do not have to think about this issue at all.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
http-walker.c

index 88da546..2c721f0 100644 (file)
@@ -29,7 +29,7 @@ struct object_request {
 struct alternates_request {
        struct walker *walker;
        const char *base;
-       char *url;
+       struct strbuf *url;
        struct strbuf *buffer;
        struct active_request_slot *slot;
        int http_specific;
@@ -195,10 +195,11 @@ static void process_alternates_response(void *callback_data)
 
                        /* Try reusing the slot to get non-http alternates */
                        alt_req->http_specific = 0;
-                       sprintf(alt_req->url, "%s/objects/info/alternates",
-                               base);
+                       strbuf_reset(alt_req->url);
+                       strbuf_addf(alt_req->url, "%s/objects/info/alternates",
+                                   base);
                        curl_easy_setopt(slot->curl, CURLOPT_URL,
-                                        alt_req->url);
+                                        alt_req->url->buf);
                        active_requests++;
                        slot->in_use = 1;
                        if (slot->finished != NULL)
@@ -312,7 +313,7 @@ static void process_alternates_response(void *callback_data)
 static void fetch_alternates(struct walker *walker, const char *base)
 {
        struct strbuf buffer = STRBUF_INIT;
-       char *url;
+       struct strbuf url = STRBUF_INIT;
        struct active_request_slot *slot;
        struct alternates_request alt_req;
        struct walker_data *cdata = walker->data;
@@ -338,7 +339,7 @@ static void fetch_alternates(struct walker *walker, const char *base)
        if (walker->get_verbosely)
                fprintf(stderr, "Getting alternates list for %s\n", base);
 
-       url = xstrfmt("%s/objects/info/http-alternates", base);
+       strbuf_addf(&url, "%s/objects/info/http-alternates", base);
 
        /*
         * Use a callback to process the result, since another request
@@ -351,10 +352,10 @@ static void fetch_alternates(struct walker *walker, const char *base)
 
        curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
        curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
-       curl_easy_setopt(slot->curl, CURLOPT_URL, url);
+       curl_easy_setopt(slot->curl, CURLOPT_URL, url.buf);
 
        alt_req.base = base;
-       alt_req.url = url;
+       alt_req.url = &url;
        alt_req.buffer = &buffer;
        alt_req.http_specific = 1;
        alt_req.slot = slot;
@@ -365,7 +366,7 @@ static void fetch_alternates(struct walker *walker, const char *base)
                cdata->got_alternates = -1;
 
        strbuf_release(&buffer);
-       free(url);
+       strbuf_release(&url);
 }
 
 static int fetch_indices(struct walker *walker, struct alt_base *repo)