Commit 6dfc8bc2499db78eae4e29dd121c75121f0e8baf
2018-07-09T23:10:05
Merge pull request #4719 from pks-t/pks/delta-oob Delta OOB access
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 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127
diff --git a/src/delta.c b/src/delta.c
index 073cba7..b9352b8 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 0000000..5bb95a2
--- /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 711fe64..7edf37b 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"