Jeff,
We have a fix like this in the next set of transaction updates.
https://code-review.googlesource.com/#/c/1012/13/fast-import.c
However, if your concerns are the integrity of the servers and not
taking any chances
you might not want to wait for my patches to graduate.
ronnie sahlberg
On Fri, Aug 22, 2014 at 10:32 PM, Jeff King p...@peff.net wrote:
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 p...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
