On Tue, May 5, 2015 at 1:30 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > From: Eric Sunshine <sunshine@xxxxxxxxxxxxxx> > Date: Mon, 4 May 2015 03:25:15 -0400 > Subject: [PATCH] hash-object --literally: fix buffer overrun with extra-long object type > > "hash-object" learned in 5ba9a93 (hash-object: add --literally > option, 2014-09-11) to allow crafting a corrupt/broken object of > unknown type. > > When the user-provided type is particularly long, however, it can > overflow the relatively small stack-based character array handed to > write_sha1_file_prepare() by hash_sha1_file() and write_sha1_file(), > leading to stack corruption (and crash). Introduce a custom helper > to allow arbitrarily long typenames just for "hash-object --literally". > > [jc: Eric's original used a strbuf in the more common codepaths, and > I rewrote it to avoid penalizing the non-literally code. Bugs are mine] Thanks for re-rolling again. The amended Subject: works nicely now, and the addition to the commit message makes sense. Also, the code changes in response to the minor questions I raised[1] all look good. [1]: http://thread.gmane.org/gmane.comp.version-control.git/268306/focus=268374 (rest of patch left unsnipped) > Signed-off-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx> > Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> > --- > diff --git a/builtin/hash-object.c b/builtin/hash-object.c > index 6158363..17e8bfdc 100644 > --- a/builtin/hash-object.c > +++ b/builtin/hash-object.c > @@ -22,10 +22,8 @@ static int hash_literally(unsigned char *sha1, int fd, const char *type, unsigne > > if (strbuf_read(&buf, fd, 4096) < 0) > ret = -1; > - else if (flags & HASH_WRITE_OBJECT) > - ret = write_sha1_file(buf.buf, buf.len, type, sha1); > else > - ret = hash_sha1_file(buf.buf, buf.len, type, sha1); > + ret = hash_sha1_file_literally(buf.buf, buf.len, type, sha1, flags); > strbuf_release(&buf); > return ret; > } > diff --git a/cache.h b/cache.h > index dfa1a56..e037cad 100644 > --- a/cache.h > +++ b/cache.h > @@ -888,6 +888,7 @@ static inline const unsigned char *lookup_replace_object_extended(const unsigned > extern int sha1_object_info(const unsigned char *, unsigned long *); > extern int hash_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *sha1); > extern int write_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *return_sha1); > +extern int hash_sha1_file_literally(const void *buf, unsigned long len, const char *type, unsigned char *sha1, unsigned flags); > extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned char *); > extern int force_object_loose(const unsigned char *sha1, time_t mtime); > extern int git_open_noatime(const char *name); > diff --git a/sha1_file.c b/sha1_file.c > index c08c0cb..dc940e6 100644 > --- a/sha1_file.c > +++ b/sha1_file.c > @@ -2962,6 +2962,27 @@ int write_sha1_file(const void *buf, unsigned long len, const char *type, unsign > return write_loose_object(sha1, hdr, hdrlen, buf, len, 0); > } > > +int hash_sha1_file_literally(const void *buf, unsigned long len, const char *type, > + unsigned char *sha1, unsigned flags) > +{ > + char *header; > + int hdrlen, status = 0; > + > + /* type string, SP, %lu of the length plus NUL must fit this */ > + header = xmalloc(strlen(type) + 32); > + write_sha1_file_prepare(buf, len, type, sha1, header, &hdrlen); > + > + if (!(flags & HASH_WRITE_OBJECT)) > + goto cleanup; > + if (has_sha1_file(sha1)) > + goto cleanup; > + status = write_loose_object(sha1, header, hdrlen, buf, len, 0); > + > +cleanup: > + free(header); > + return status; > +} > + > int force_object_loose(const unsigned char *sha1, time_t mtime) > { > void *buf; > -- > 2.4.0-311-gf1d9b8d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html