path: root/http-walker.c
diff options
authorJeff King <>2017-03-13 14:04:45 (GMT)
committerJunio C Hamano <>2017-03-13 17:20:29 (GMT)
commitd61434ae813cc86a1a87d05cc61e36e87b0e20a9 (patch)
tree83b452df8faf0dda505a231abe267a89de1a32d3 /http-walker.c
parente7e07d5a4fcc2a203d9873968ad3e6bd4d7419d7 (diff)
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 <> Signed-off-by: Junio C Hamano <>
Diffstat (limited to 'http-walker.c')
1 files changed, 7 insertions, 4 deletions
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 @@ static void process_alternates_response(void *callback_data)
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",
newalt = xmalloc(sizeof(*newalt));