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>
diff --git a/http-walker.c b/http-walker.c
index b34b6ac..507c200 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -296,13 +296,16 @@
okay = 1;
}
}
- /* skip "objects\n" at end */
if (okay) {
struct strbuf target = STRBUF_INIT;
strbuf_add(&target, base, serverlen);
- strbuf_add(&target, data + i, posn - i - 7);
-
- if (is_alternate_allowed(target.buf)) {
+ strbuf_add(&target, data + i, posn - i);
+ if (!strbuf_strip_suffix(&target, "objects")) {
+ warning("ignoring alternate that does"
+ " not end in 'objects': %s",
+ target.buf);
+ strbuf_release(&target);
+ } else if (is_alternate_allowed(target.buf)) {
warning("adding alternate object store: %s",
target.buf);
newalt = xmalloc(sizeof(*newalt));
