From: Junio C Hamano <gitster@pobox•com>
To: Eric Sunshine <sunshine@sunshineco•com>
Cc: Git List <git@vger•kernel.org>
Subject: Re: [PATCH 2/4] write_sha1_file_prepare: fix buffer overrun with extra-long object type
Date: Tue, 05 May 2015 10:30:09 -0700 [thread overview]
Message-ID: <xmqqvbg7dja6.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CAPig+cS3f2XggxqbvX6Z2Da24QKLOg915Bf-bTVa+4oVzfhA1A@mail.gmail.com> (Eric Sunshine's message of "Mon, 4 May 2015 20:13:18 -0400")
Eric Sunshine <sunshine@sunshineco•com> writes:
> On Mon, May 4, 2015 at 5:37 PM, Junio C Hamano <gitster@pobox•com> wrote:
>> From: Eric Sunshine <sunshine@sunshineco•com>
>
> Thanks for re-rerolling this series. Considering that the only bits
> left from me are the diagnosis and the (mostly intact) commit message,
> perhaps the authorship should be changed, or at the very least a big
> "Helped-by: Junio" added? Anyhow, a few minor comments below...
I am a bit too lazy to take the ownership, so I decided only to take
the blame ;-)
Here is a replacement; all the other patches stay the same.
-- >8 --
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>
---
builtin/hash-object.c | 4 +---
cache.h | 1 +
sha1_file.c | 21 +++++++++++++++++++++
3 files changed, 23 insertions(+), 3 deletions(-)
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
next prev parent reply other threads:[~2015-05-05 17:30 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-04 7:25 [PATCH 0/3] hash-object crash fix and new doc and tests Eric Sunshine
2015-05-04 7:25 ` [PATCH 1/3] git-hash-object.txt: document --literally option Eric Sunshine
2015-05-04 7:25 ` [PATCH 2/3] t1007: add hash-object --literally tests Eric Sunshine
2015-05-04 7:25 ` [PATCH 3/3] write_sha1_file_prepare: fix buffer overrun with extra-long object type Eric Sunshine
2015-05-04 17:58 ` Junio C Hamano
2015-05-04 17:59 ` Junio C Hamano
2015-05-04 21:37 ` [PATCH 0/4] "hash-object --literally" fixes Junio C Hamano
2015-05-04 21:37 ` [PATCH 1/4] git-hash-object.txt: document --literally option Junio C Hamano
2015-05-04 21:37 ` [PATCH 2/4] write_sha1_file_prepare: fix buffer overrun with extra-long object type Junio C Hamano
2015-05-05 0:13 ` Eric Sunshine
2015-05-05 0:28 ` Junio C Hamano
2015-05-05 17:30 ` Junio C Hamano [this message]
2015-05-05 18:49 ` Eric Sunshine
2015-05-04 21:37 ` [PATCH 3/4] t1007: add hash-object --literally tests Junio C Hamano
2015-05-04 21:37 ` [PATCH 4/4] write_sha1_file(): do not use a separate sha1[] array Junio C Hamano
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=xmqqvbg7dja6.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=sunshine@sunshineco$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox