http-walker: fix buffer underflow processing remote alternates

If we parse a remote alternates (or http-alternates), we expect relative lines like: ../../foo.git/objects which we convert into "$URL/../foo.git/" (and then use that as a base for fetching more objects). But if the remote feeds us nonsense like just: ../ we will try to blindly strip the last 7 characters, assuming they contain the string "objects". Since we don't _have_ 7 characters at all, this results in feeding a small negative value to strbuf_add(), which converts it to a size_t, resulting in a big positive value. This should consistently fail (since we can't generall allocate the max size_t minus 7 bytes), so there shouldn't be any security implications. Let's fix this by using strbuf_strip_suffix() to drop the characters we want. If they're not present, we'll ignore the alternate (in theory we could use it as-is, but the rest of the http-walker code unconditionally tacks "objects/" back on, so it is it not prepared to handle such a case). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Mar 13, 2017 at 10:04 UTC d61434ae813cc86a1a87d05cc61e36e87b0e20a9
1 file changed +7 -4
http-walker.c
+7 -4
@@ -296,13 +296,16 @@ static void process_alternates_response(void *callback_data)
296 okay = 1;
297 }
298 }
299 - /* skip "objects\n" at end */
299 if (okay) {
300 struct strbuf target = STRBUF_INIT;
301 strbuf_add(&target, base, serverlen);
303 - strbuf_add(&target, data + i, posn - i - 7);
304 -
305 - if (is_alternate_allowed(target.buf)) {
302 + strbuf_add(&target, data + i, posn - i);
303 + if (!strbuf_strip_suffix(&target, "objects")) {
304 + warning("ignoring alternate that does"
305 + " not end in 'objects': %s",
306 + target.buf);
307 + strbuf_release(&target);
308 + } else if (is_alternate_allowed(target.buf)) {
309 warning("adding alternate object store: %s",
310 target.buf);
311 newalt = xmalloc(sizeof(*newalt));