http-walker: store url in a strbuf

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>

Jeff King committed Sep 24, 2015 at 17:07 UTC 54ba4c5fa2d7de216ca090ac2e657728462c81d5
1 file changed +10 -9
http-walker.c
+10 -9
@@ -29,7 +29,7 @@ struct object_request {
29 struct alternates_request {
30 struct walker *walker;
31 const char *base;
32 - char *url;
32 + struct strbuf *url;
33 struct strbuf *buffer;
34 struct active_request_slot *slot;
35 int http_specific;
@@ -195,10 +195,11 @@ static void process_alternates_response(void *callback_data)
195
196 /* Try reusing the slot to get non-http alternates */
197 alt_req->http_specific = 0;
198 - sprintf(alt_req->url, "%s/objects/info/alternates",
199 - base);
198 + strbuf_reset(alt_req->url);
199 + strbuf_addf(alt_req->url, "%s/objects/info/alternates",
200 + base);
201 curl_easy_setopt(slot->curl, CURLOPT_URL,
201 - alt_req->url);
202 + alt_req->url->buf);
203 active_requests++;
204 slot->in_use = 1;
205 if (slot->finished != NULL)
@@ -312,7 +313,7 @@ static void process_alternates_response(void *callback_data)
313 static void fetch_alternates(struct walker *walker, const char *base)
314 {
315 struct strbuf buffer = STRBUF_INIT;
315 - char *url;
316 + struct strbuf url = STRBUF_INIT;
317 struct active_request_slot *slot;
318 struct alternates_request alt_req;
319 struct walker_data *cdata = walker->data;
@@ -338,7 +339,7 @@ static void fetch_alternates(struct walker *walker, const char *base)
339 if (walker->get_verbosely)
340 fprintf(stderr, "Getting alternates list for %s\n", base);
341
341 - url = xstrfmt("%s/objects/info/http-alternates", base);
342 + strbuf_addf(&url, "%s/objects/info/http-alternates", base);
343
344 /*
345 * Use a callback to process the result, since another request
@@ -351,10 +352,10 @@ static void fetch_alternates(struct walker *walker, const char *base)
352
353 curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer);
354 curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer);
354 - curl_easy_setopt(slot->curl, CURLOPT_URL, url);
355 + curl_easy_setopt(slot->curl, CURLOPT_URL, url.buf);
356
357 alt_req.base = base;
357 - alt_req.url = url;
358 + alt_req.url = &url;
359 alt_req.buffer = &buffer;
360 alt_req.http_specific = 1;
361 alt_req.slot = slot;
@@ -365,7 +366,7 @@ static void fetch_alternates(struct walker *walker, const char *base)
366 cdata->got_alternates = -1;
367
368 strbuf_release(&buffer);
368 - free(url);
369 + strbuf_release(&url);
370 }
371
372 static int fetch_indices(struct walker *walker, struct alt_base *repo)