Merge pull request #4717 from pks-t/pks/v0.27.3 Release v0.27.3
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 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1c6a5eb..8b149ee 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 c1b2042..8c2594f 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 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 173a599..c17ba5e 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"