Commit 24597812220325f1cb38f61e56b095ff38d1b1cb
2018-06-29T09:11:02
delta: fix out-of-bounds read of delta When computing the offset and length of the delta base, we repeatedly increment the `delta` pointer without checking whether we have advanced past its end already, which can thus result in an out-of-bounds read. Fix this by repeatedly checking whether we have reached the end. Add a test which would cause Valgrind to produce an error. Reported-by: Riccardo Schirone <rschiron@redhat.com> Test-provided-by: Riccardo Schirone <rschiron@redhat.com>
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47
diff --git a/src/delta.c b/src/delta.c
index 8d9e614..8676e7a 100644
--- a/src/delta.c
+++ b/src/delta.c
@@ -568,15 +568,17 @@ int git_delta_apply(
/* 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 |= ((unsigned) *delta++ << 24UL);
-
- if (cmd & 0x10) len = *delta++;
- if (cmd & 0x20) len |= *delta++ << 8UL;
- if (cmd & 0x40) len |= *delta++ << 16UL;
+#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 (base_len < off + len || res_sz < len)
goto fail;
diff --git a/tests/delta/apply.c b/tests/delta/apply.c
index 24513e0..5bb95a2 100644
--- a/tests/delta/apply.c
+++ b/tests/delta/apply.c
@@ -10,3 +10,12 @@ void test_delta_apply__read_at_off(void)
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)));
+}