All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Ronnie Sahlberg <sahlberg@google.com>,
	Michael Haggerty <mhagger@alum.mit.edu>
Subject: [PATCH 4/5] fast-import: fix buffer overflow in dump_tags
Date: Sat, 23 Aug 2014 01:32:37 -0400	[thread overview]
Message-ID: <20140823053237.GD18075@peff.net> (raw)
In-Reply-To: <20140823052334.GA17813@peff.net>

When creating a new annotated tag, we sprintf the refname
into a static-sized buffer. If we have an absurdly long
tagname, like:

  git init repo &&
  cd repo &&
  git commit --allow-empty -m foo &&
  git tag -m message mytag &&
  git fast-export mytag |
  perl -lpe '/^tag/ and s/mytag/"a" x 8192/e' |
  git fast-import <input

we'll overflow the buffer. We can fix it by using a strbuf.

Signed-off-by: Jeff King <peff@peff.net>
---
I'm not sure how easily exploitable this is. The buffer is on the stack,
and we definitely demolish the return address. But we never actually
return from the function, since lock_ref_sha1 will fail in such a case
and die (it cannot succeed because the name is longer than PATH_MAX,
which we check when concatenating it with $GIT_DIR).

Still, there is no limit to the size of buffer you can feed it, so it's
entirely possible you can overwrite something else and cause some
mischief. So I wouldn't call it trivially exploitable, but I would not
bet my servers that it is not (and of course it is easy to trigger if
you can convince somebody to run fast-import a stream, so any remote
helpers produce a potentially vulnerable situation).

 fast-import.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index f25a4ae..a1479e9 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1734,14 +1734,16 @@ static void dump_tags(void)
 	static const char *msg = "fast-import";
 	struct tag *t;
 	struct ref_lock *lock;
-	char ref_name[PATH_MAX];
+	struct strbuf ref_name = STRBUF_INIT;
 
 	for (t = first_tag; t; t = t->next_tag) {
-		sprintf(ref_name, "tags/%s", t->name);
-		lock = lock_ref_sha1(ref_name, NULL);
+		strbuf_reset(&ref_name);
+		strbuf_addf(&ref_name, "tags/%s", t->name);
+		lock = lock_ref_sha1(ref_name.buf, NULL);
 		if (!lock || write_ref_sha1(lock, t->sha1, msg) < 0)
-			failure |= error("Unable to update %s", ref_name);
+			failure |= error("Unable to update %s", ref_name.buf);
 	}
+	strbuf_release(&ref_name);
 }
 
 static void dump_marks_helper(FILE *f,
-- 
2.1.0.346.ga0367b9

  parent reply	other threads:[~2014-08-23  5:32 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-23  5:23 [PATCH 0/5] fix pack-refs pruning of refs/foo Jeff King
2014-08-23  5:26 ` [PATCH 1/5] git-prompt: do not look for refs/stash in $GIT_DIR Jeff King
2014-08-23  5:27 ` [PATCH 2/5] pack-refs: prune top-level refs like "refs/foo" Jeff King
2014-08-23  5:27 ` [PATCH 3/5] fast-import: clean up pack_data pointer in end_packfile Jeff King
2014-08-25 17:16   ` Ronnie Sahlberg
2014-08-25 18:55     ` Jeff King
2014-08-23  5:32 ` Jeff King [this message]
2014-08-25 17:11   ` [PATCH 4/5] fast-import: fix buffer overflow in dump_tags Ronnie Sahlberg
2014-08-23  5:33 ` [PATCH 5/5] fast-import: stop using lock_ref_sha1 Jeff King
2014-08-25 17:22   ` Ronnie Sahlberg
2014-08-23  7:29 ` [PATCH 0/5] fix pack-refs pruning of refs/foo Michael Haggerty
2014-08-23  7:46   ` Jeff King
2014-08-25 17:38 ` Ronnie Sahlberg
2014-08-25 18:56   ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140823053237.GD18075@peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=mhagger@alum.mit.edu \
    --cc=sahlberg@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.