attr: ignore overly large gitattributes files

Similar as with the preceding commit, start ignoring gitattributes files
that are overly large to protect us against out-of-bounds reads and
writes caused by integer overflows. Unfortunately, we cannot just define
"overly large" in terms of any preexisting limits in the codebase.

Instead, we choose a very conservative limit of 100MB. This is plenty of
room for specifying gitattributes, and incidentally it is also the limit
for blob sizes for GitHub. While we don't want GitHub to dictate limits
here, it is still sensible to use this fact for an informed decision
given that it is hosting a huge set of repositories. Furthermore, over
at GitLab we scanned a subset of repositories for their root-level
attribute files. We found that 80% of them have a gitattributes file
smaller than 100kB, 99.99% have one smaller than 1MB, and only a single
repository had one that was almost 3MB in size. So enforcing a limit of
100MB seems to give us ample of headroom.

With this limit in place we can be reasonably sure that there is no easy
way to exploit the gitattributes file via integer overflows anymore.
Furthermore, it protects us against resource exhaustion caused by
allocating the in-memory data structures required to represent the
parsed attributes.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/attr.c b/attr.c
index 38ecd2fff30d33a86ec11f16f454a0a504a15fa2..f9316d14baf835ee498722a4dc26bd9bd56a8efd 100644 (file)
--- a/attr.c
+++ b/attr.c
@@ -708,10 +708,25 @@ static struct attr_stack *read_attr_from_file(const char *path, int macro_ok)
        FILE *fp = fopen_or_warn(path, "r");
        struct attr_stack *res;
        int lineno = 0;
+       int fd;
+       struct stat st;
 
        if (!fp)
                return NULL;
-       res = xcalloc(1, sizeof(*res));
+
+       fd = fileno(fp);
+       if (fstat(fd, &st)) {
+               warning_errno(_("cannot fstat gitattributes file '%s'"), path);
+               fclose(fp);
+               return NULL;
+       }
+       if (st.st_size >= ATTR_MAX_FILE_SIZE) {
+               warning(_("ignoring overly large gitattributes file '%s'"), path);
+               fclose(fp);
+               return NULL;
+       }
+
+       CALLOC_ARRAY(res, 1);
        while (strbuf_getline(&buf, fp) != EOF) {
                if (!lineno && starts_with(buf.buf, utf8_bom))
                        strbuf_remove(&buf, 0, strlen(utf8_bom));
@@ -730,13 +745,18 @@ static struct attr_stack *read_attr_from_index(const struct index_state *istate,
        struct attr_stack *res;
        char *buf, *sp;
        int lineno = 0;
+       size_t size;
 
        if (!istate)
                return NULL;
 
-       buf = read_blob_data_from_index(istate, path, NULL);
+       buf = read_blob_data_from_index(istate, path, &size);
        if (!buf)
                return NULL;
+       if (size >= ATTR_MAX_FILE_SIZE) {
+               warning(_("ignoring overly large gitattributes blob '%s'"), path);
+               return NULL;
+       }
 
        res = xcalloc(1, sizeof(*res));
        for (sp = buf; *sp; ) {

diff --git a/attr.h b/attr.h
index df9a75da550eb7f0fcd627dfee2e7e5ed098ae70..5970f930fd0a44b4784964d0c7d1d3e882b84878 100644 (file)
--- a/attr.h
+++ b/attr.h
@@ -113,6 +113,12 @@
  */
 #define ATTR_MAX_LINE_LENGTH 2048
 
+ /**
+  * The maximum size of the giattributes file. If the file exceeds this size we
+  * will ignore it.
+  */
+#define ATTR_MAX_FILE_SIZE (100 * 1024 * 1024)
+
 struct index_state;
 
 /**

diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index 7d68e6a56e964dda5f59b4da790b414a97611a0d..9d9aa2855d226feed852bfe968d8d1a27fcf447e 100755 (executable)
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -361,6 +361,14 @@ test_expect_success 'large attributes line ignores trailing content in tree' '
        test_must_be_empty actual
 '
 
+test_expect_success EXPENSIVE 'large attributes file ignored in tree' '
+       test_when_finished "rm .gitattributes" &&
+       dd if=/dev/zero of=.gitattributes bs=101M count=1 2>/dev/null &&
+       git check-attr --all path >/dev/null 2>err &&
+       echo "warning: ignoring overly large gitattributes file ${SQ}.gitattributes${SQ}" >expect &&
+       test_cmp expect err
+'
+
 test_expect_success 'large attributes line ignored in index' '
        test_when_finished "git update-index --remove .gitattributes" &&
        blob=$(printf "path %02043d" 1 | git hash-object -w --stdin) &&
@@ -381,4 +389,13 @@ test_expect_success 'large attributes line ignores trailing content in index' '
        test_must_be_empty actual
 '
 
+test_expect_success EXPENSIVE 'large attributes file ignored in index' '
+       test_when_finished "git update-index --remove .gitattributes" &&
+       blob=$(dd if=/dev/zero bs=101M count=1 2>/dev/null | git hash-object -w --stdin) &&
+       git update-index --add --cacheinfo 100644,$blob,.gitattributes &&
+       git check-attr --cached --all path >/dev/null 2>err &&
+       echo "warning: ignoring overly large gitattributes blob ${SQ}.gitattributes${SQ}" >expect &&
+       test_cmp expect err
+'
+
 test_done