Re: [PATCH 2/4] write_sha1_file_prepare: fix buffer overrun with extra-long object type

> From: Eric Sunshine <sunshine@sunshineco.com>
> 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]

> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> 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