Merge pull request #4719 from pks-t/pks/delta-oob

Delta OOB access

3 files changed, 52 insertions, 28 deletions

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 711fe6473..7edf37b51 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"