xdiff: reject files larger than ~1GB
authorJeff King <peff@peff.net>
Thu, 24 Sep 2015 23:12:45 +0000 (19:12 -0400)
committerJunio C Hamano <gitster@pobox.com>
Mon, 28 Sep 2015 21:57:23 +0000 (14:57 -0700)
The xdiff code is not prepared to handle extremely large
files. It uses "int" in many places, which can overflow if
we have a very large number of lines or even bytes in our
input files. This can cause us to produce incorrect diffs,
with no indication that the output is wrong. Or worse, we
may even underallocate a buffer whose size is the result of
an overflowing addition.

We're much better off to tell the user that we cannot diff
or merge such a large file. This patch covers both cases,
but in slightly different ways:

1. For merging, we notice the large file and cleanly fall
back to a binary merge (which is effectively "we cannot
merge this").

2. For diffing, we make the binary/text distinction much
earlier, and in many different places. For this case,
we'll use the xdi_diff as our choke point, and reject
any diff there before it hits the xdiff code.

This means in most cases we'll die() immediately after.
That's not ideal, but in practice we shouldn't
generally hit this code path unless the user is trying
to do something tricky. We already consider files
larger than core.bigfilethreshold to be binary, so this
code would only kick in when that is circumvented
(either by bumping that value, or by using a
.gitattribute to mark a file as diffable).

In other words, we can avoid being "nice" here, because
there is already nice code that tries to do the right
thing. We are adding the suspenders to the nice code's
belt, so notice when it has been worked around (both to
protect the user from malicious inputs, and because it
is better to die() than generate bogus output).

The maximum size was chosen after experimenting with feeding
large files to the xdiff code. It's just under a gigabyte,
which leaves room for two obvious cases:

- a diff3 merge conflict result on files of maximum size X
could be 3*X plus the size of the markers, which would
still be only about 3G, which fits in a 32-bit int.

- some of the diff code allocates arrays of one int per
record. Even if each file consists only of blank lines,
then a file smaller than 1G will have fewer than 1G
records, and therefore the int array will fit in 4G.

Since the limit is arbitrary anyway, I chose to go under a
gigabyte, to leave a safety margin (e.g., we would not want
to overflow by allocating "(records + 1) * sizeof(int)" or
similar.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
ll-merge.c
xdiff-interface.c
xdiff-interface.h
index 8ea03e536a56655ff48f4fa8a3050c0225d52f38..4e789f533043c78916b4281ac8113f1e75d342b4 100644 (file)
@@ -88,7 +88,10 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused,
        xmparam_t xmp;
        assert(opts);
 
-       if (buffer_is_binary(orig->ptr, orig->size) ||
+       if (orig->size > MAX_XDIFF_SIZE ||
+           src1->size > MAX_XDIFF_SIZE ||
+           src2->size > MAX_XDIFF_SIZE ||
+           buffer_is_binary(orig->ptr, orig->size) ||
            buffer_is_binary(src1->ptr, src1->size) ||
            buffer_is_binary(src2->ptr, src2->size)) {
                return ll_binary_merge(drv_unused, result,
index ecfa05f616f4b72d65bcb129c1ee2141cf3d1c47..cb67c1c42b35e412dccf9a13ad18dde727ab8ce6 100644 (file)
@@ -131,6 +131,9 @@ int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t co
        mmfile_t a = *mf1;
        mmfile_t b = *mf2;
 
+       if (mf1->size > MAX_XDIFF_SIZE || mf2->size > MAX_XDIFF_SIZE)
+               return -1;
+
        trim_common_tail(&a, &b, xecfg->ctxlen);
 
        return xdl_diff(&a, &b, xpp, xecfg, xecb);
index eff7762ee1a1bb0ea648c60a07389e22e9a1ac07..fbb5a1c3949b6ef6ba0dfb758723a48f3b402190 100644 (file)
@@ -3,6 +3,13 @@
 
 #include "xdiff/xdiff.h"
 
+/*
+ * xdiff isn't equipped to handle content over a gigabyte;
+ * we make the cutoff 1GB - 1MB to give some breathing
+ * room for constant-sized additions (e.g., merge markers)
+ */
+#define MAX_XDIFF_SIZE (1024UL * 1024 * 1023)
+
 typedef void (*xdiff_emit_consume_fn)(void *, char *, unsigned long);
 
 int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t const *xecfg, xdemitcb_t *ecb);