[PATCH 0/4] Fix various integer overflows

DORMANTno replies

6 messages, 2 authors, 2016-06-15 · open the first message on its own page

[PATCH 0/4] Fix various integer overflows

From: Ilari Liusvaara <hidden>
Date: 2016-06-15 22:48:06

Fix integer overflows in patch_delta(), unpack_sha1_rest() and
unpack_compressed_entry().

These at least can cause git to segfault, possibly worse. Operations
that cause integer overflow are not possible to do (even whole virtual
memory space would not be sufficient), so die() instead.

Ilari Liusvaara (4):
  Add xmallocz()
  Fix integer overflow in patch_delta()
  Fix integer overflow in unpack_sha1_rest()
  Fix integer overflow in unpack_compressed_entry()

 git-compat-util.h |    1 +
 patch-delta.c     |    3 +--
 sha1_file.c       |    5 ++---
 wrapper.c         |   12 +++++++++++-
 4 files changed, 15 insertions(+), 6 deletions(-)

[PATCH 1/4] Add xmallocz()

From: Ilari Liusvaara <hidden>
Date: 2016-06-15 22:48:06

Add routine for allocating NUL-terminated memory block without risking
integer overflow in addition of +1 for NUL byte.

Signed-off-by: Ilari Liusvaara <redacted>
---
 git-compat-util.h |    1 +
 wrapper.c         |   12 +++++++++++-
 2 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index 620a7c6..a3c4537 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h

@@ -348,6 +348,7 @@ extern void release_pack_memory(size_t, int);
 
 extern char *xstrdup(const char *str);
 extern void *xmalloc(size_t size);
+extern void *xmallocz(size_t size);
 extern void *xmemdupz(const void *data, size_t len);
 extern char *xstrndup(const char *str, size_t len);
 extern void *xrealloc(void *ptr, size_t size);

diff --git a/wrapper.c b/wrapper.c
index c9be140..dd7b6ee 100644
--- a/wrapper.c
+++ b/wrapper.c

@@ -34,6 +34,16 @@ void *xmalloc(size_t size)
 	return ret;
 }
 
+void *xmallocz(size_t size)
+{
+	void *ret;
+	if (size + 1 < size)
+		die("Data too large to fit into virtual memory space.");
+	ret = xmalloc(size + 1);
+	((char*)ret)[size] = 0;
+	return ret;
+}
+
 /*
  * xmemdupz() allocates (len + 1) bytes of memory, duplicates "len" bytes of
  * "data" to the allocated memory, zero terminates the allocated memory,

@@ -42,7 +52,7 @@ void *xmalloc(size_t size)
  */
 void *xmemdupz(const void *data, size_t len)
 {
-	char *p = xmalloc(len + 1);
+	char *p = xmallocz(len);
 	memcpy(p, data, len);
 	p[len] = '\0';
 	return p;

-- 
1.6.6.1.439.gf06b6

[PATCH 3/4] Fix integer overflow in unpack_sha1_rest()

From: Ilari Liusvaara <hidden>
Date: 2016-06-15 22:48:06

Signed-off-by: Ilari Liusvaara <redacted>
---
 sha1_file.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 12478a3..39f0844 100644
--- a/sha1_file.c
+++ b/sha1_file.c

@@ -1166,7 +1166,7 @@ static int unpack_sha1_header(z_stream *stream, unsigned char *map, unsigned lon
 static void *unpack_sha1_rest(z_stream *stream, void *buffer, unsigned long size, const unsigned char *sha1)
 {
 	int bytes = strlen(buffer) + 1;
-	unsigned char *buf = xmalloc(1+size);
+	unsigned char *buf = xmallocz(size);
 	unsigned long n;
 	int status = Z_OK;

-- 
1.6.6.1.439.gf06b6

[PATCH 2/4] Fix integer overflow in patch_delta()

From: Ilari Liusvaara <hidden>
Date: 2016-06-15 22:48:06

Signed-off-by: Ilari Liusvaara <redacted>
---
 patch-delta.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/patch-delta.c b/patch-delta.c
index e02e13b..d218faa 100644
--- a/patch-delta.c
+++ b/patch-delta.c

@@ -33,8 +33,7 @@ void *patch_delta(const void *src_buf, unsigned long src_size,
 
 	/* now the result size */
 	size = get_delta_hdr_size(&data, top);
-	dst_buf = xmalloc(size + 1);
-	dst_buf[size] = 0;
+	dst_buf = xmallocz(size);
 
 	out = dst_buf;
 	while (data < top) {

-- 
1.6.6.1.439.gf06b6

[PATCH 4/4] Fix integer overflow in unpack_compressed_entry()

From: Ilari Liusvaara <hidden>
Date: 2016-06-15 22:48:06

Signed-off-by: Ilari Liusvaara <redacted>
---
 sha1_file.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 39f0844..ea2ea75 100644
--- a/sha1_file.c
+++ b/sha1_file.c

@@ -1517,8 +1517,7 @@ static void *unpack_compressed_entry(struct packed_git *p,
 	z_stream stream;
 	unsigned char *buffer, *in;
 
-	buffer = xmalloc(size + 1);
-	buffer[size] = 0;
+	buffer = xmallocz(size);
 	memset(&stream, 0, sizeof(stream));
 	stream.next_out = buffer;
 	stream.avail_out = size + 1;

-- 
1.6.6.1.439.gf06b6

Re: [PATCH 1/4] Add xmallocz()

From: Bill Lear <hidden>
Date: 2016-06-15 22:48:06

On Tuesday, January 26, 2010 at 20:24:12 (+0200) Ilari Liusvaara writes:

Add routine for allocating NUL-terminated memory block without risking
integer overflow in addition of +1 for NUL byte.
...
void *xmemdupz(const void *data, size_t len)
{
-	char *p = xmalloc(len + 1);
+	char *p = xmallocz(len);
	memcpy(p, data, len);
	p[len] = '\0';
	return p;

Do you need the statement

 	p[len] = '\0';

any longer in the above?  If not, could you just do this:

void *xmemdupz(const void *data, size_t len)
{
	return memcpy(xmallocz(len), data, len);
}


??


Bill

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help