Merge pull request #4717 from pks-t/pks/v0.27.3v0.27.3

Release v0.27.3

5 files changed, 77 insertions, 30 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1c6a5eb00..8b149ee4c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,26 @@
+v0.27.3
+-------
+
+This is a security release fixing out-of-bounds reads when
+reading objects from a packfile. This corresponds to
+CVE-2018-10887 and CVE-2018-10888, which were both reported by
+Riccardo Schirone.
+
+When packing objects into a single so-called packfile, objects
+may not get stored as complete copies but instead as deltas
+against another object "base". A specially crafted delta object
+could trigger an integer overflow and thus bypass our input
+validation, which may result in copying memory before or after
+the base object into the final deflated object. This may lead to
+objects containing copies of system memory being written into the
+object database. As the hash of those objects cannot be easily
+controlled by the attacker, it is unlikely that any of those
+objects will be valid and referenced by the commit graph.
+
+Note that the error could also be triggered by the function
+`git_apply__patch`. But as this function is not in use outside of
+our test suite, it is not a possible attack vector.
+
 v0.27.2
 ---------
 
diff --git a/include/git2/version.h b/include/git2/version.h
index c1b204227..8c2594f50 100644
--- a/include/git2/version.h
+++ b/include/git2/version.h
@@ -7,10 +7,10 @@
 #ifndef INCLUDE_git_version_h__
 #define INCLUDE_git_version_h__
 
-#define LIBGIT2_VERSION "0.27.2"
+#define LIBGIT2_VERSION "0.27.3"
 #define LIBGIT2_VER_MAJOR 0
 #define LIBGIT2_VER_MINOR 27
-#define LIBGIT2_VER_REVISION 2
+#define LIBGIT2_VER_REVISION 3
 #define LIBGIT2_VER_PATCH 0
 
 #define LIBGIT2_SOVERSION 27
diff --git a/src/delta.c b/src/delta.c
index 073cba7c6..b9352b8ee 100644
--- a/src/delta.c
+++ b/src/delta.c
@@ -539,10 +539,11 @@ int git_delta_apply(
 	*out = NULL;
 	*out_len = 0;
 
-	/* Check that the base size matches the data we were given;
-	* if not we would underflow while accessing data from the
-	* base object, resulting in data corruption or segfault.
-	*/
+	/*
+	 * Check that the base size matches the data we were given;
+	 * if not we would underflow while accessing data from the
+	 * base object, resulting in data corruption or segfault.
+	 */
 	if ((hdr_sz(&base_sz, &delta, delta_end) < 0) || (base_sz != base_len)) {
 		giterr_set(GITERR_INVALID, "failed to apply delta: base size does not match given data");
 		return -1;
@@ -564,31 +565,34 @@ int git_delta_apply(
 	while (delta < delta_end) {
 		unsigned char cmd = *delta++;
 		if (cmd & 0x80) {
-			/* cmd is a copy instruction; copy from the base.
-			*/
-			size_t off = 0, len = 0;
-
-			if (cmd & 0x01) off = *delta++;
-			if (cmd & 0x02) off |= *delta++ << 8UL;
-			if (cmd & 0x04) off |= *delta++ << 16UL;
-			if (cmd & 0x08) off |= *delta++ << 24UL;
-
-			if (cmd & 0x10) len = *delta++;
-			if (cmd & 0x20) len |= *delta++ << 8UL;
-			if (cmd & 0x40) len |= *delta++ << 16UL;
-			if (!len)		len = 0x10000;
-
-			if (base_len < off + len || res_sz < len)
+			/* cmd is a copy instruction; copy from the base. */
+			size_t off = 0, len = 0, end;
+
+#define ADD_DELTA(o, shift) { if (delta < delta_end) (o) |= ((unsigned) *delta++ << shift); else goto fail; }
+			if (cmd & 0x01) ADD_DELTA(off, 0UL);
+			if (cmd & 0x02) ADD_DELTA(off, 8UL);
+			if (cmd & 0x04) ADD_DELTA(off, 16UL);
+			if (cmd & 0x08) ADD_DELTA(off, 24UL);
+
+			if (cmd & 0x10) ADD_DELTA(len, 0UL);
+			if (cmd & 0x20) ADD_DELTA(len, 8UL);
+			if (cmd & 0x40) ADD_DELTA(len, 16UL);
+			if (!len)       len = 0x10000;
+#undef ADD_DELTA
+
+			if (GIT_ADD_SIZET_OVERFLOW(&end, off, len) ||
+			    base_len < end || res_sz < len)
 				goto fail;
+
 			memcpy(res_dp, base + off, len);
 			res_dp += len;
 			res_sz -= len;
 
-		}
-		else if (cmd) {
-			/* cmd is a literal insert instruction; copy from
-			* the delta stream itself.
-			*/
+		} else if (cmd) {
+			/*
+			 * cmd is a literal insert instruction; copy from
+			 * the delta stream itself.
+			 */
 			if (delta_end - delta < cmd || res_sz < cmd)
 				goto fail;
 			memcpy(res_dp, delta, cmd);
@@ -596,10 +600,8 @@ int git_delta_apply(
 			res_dp += cmd;
 			res_sz -= cmd;
 
-		}
-		else {
-			/* cmd == 0 is reserved for future encodings.
-			*/
+		} else {
+			/* cmd == 0 is reserved for future encodings. */
 			goto fail;
 		}
 	}
diff --git a/tests/delta/apply.c b/tests/delta/apply.c
new file mode 100644
index 000000000..5bb95a283
--- /dev/null
+++ b/tests/delta/apply.c
@@ -0,0 +1,21 @@
+#include "clar_libgit2.h"
+
+#include "delta.h"
+
+void test_delta_apply__read_at_off(void)
+{
+	unsigned char base[16] = { 0 }, delta[] = { 0x10, 0x10, 0xff, 0xff, 0xff, 0xff, 0xff, 0x10, 0x00, 0x00 };
+	void *out;
+	size_t outlen;
+
+	cl_git_fail(git_delta_apply(&out, &outlen, base, sizeof(base), delta, sizeof(delta)));
+}
+
+void test_delta_apply__read_after_limit(void)
+{
+	unsigned char base[16] = { 0 }, delta[] = { 0x10, 0x70, 0xff };
+	void *out;
+	size_t outlen;
+
+	cl_git_fail(git_delta_apply(&out, &outlen, base, sizeof(base), delta, sizeof(delta)));
+}
diff --git a/tests/diff/binary.c b/tests/diff/binary.c
index 173a5994e..c17ba5ef4 100644
--- a/tests/diff/binary.c
+++ b/tests/diff/binary.c
@@ -3,6 +3,7 @@
 #include "git2/sys/diff.h"
 
 #include "buffer.h"
+#include "delta.h"
 #include "filebuf.h"
 #include "repository.h"