@matheusbsbs / matheus-bagliano / commits / 3c50032ff5

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>

Patrick Steinhardt committed Dec 1, 2022 at 15:45 UTC 3c50032ff5289cc45659f21949c8d09e52164579

3 files changed +45 -2

attr.c

+22 -2

@@ -708,10 +708,25 @@ static struct attr_stack *read_attr_from_file(const char *path, int macro_ok)

708 FILE *fp = fopen_or_warn(path, "r");

709 struct attr_stack *res;

710 int lineno = 0;

711 + int fd;

712 + struct stat st;

713

714 if (!fp)

715 return NULL;

714 - res = xcalloc(1, sizeof(*res));

716 +

717 + fd = fileno(fp);

718 + if (fstat(fd, &st)) {

719 + warning_errno(_("cannot fstat gitattributes file '%s'"), path);

720 + fclose(fp);

721 + return NULL;

722 + }

723 + if (st.st_size >= ATTR_MAX_FILE_SIZE) {

724 + warning(_("ignoring overly large gitattributes file '%s'"), path);

725 + fclose(fp);

726 + return NULL;

727 + }

728 +

729 + CALLOC_ARRAY(res, 1);

730 while (strbuf_getline(&buf, fp) != EOF) {

731 if (!lineno && starts_with(buf.buf, utf8_bom))

732 strbuf_remove(&buf, 0, strlen(utf8_bom));

@@ -730,13 +745,18 @@ static struct attr_stack *read_attr_from_index(const struct index_state *istate,

745 struct attr_stack *res;

746 char *buf, *sp;

747 int lineno = 0;

748 + size_t size;

749

750 if (!istate)

751 return NULL;

752

737 - buf = read_blob_data_from_index(istate, path, NULL);

753 + buf = read_blob_data_from_index(istate, path, &size);

754 if (!buf)

755 return NULL;

756 + if (size >= ATTR_MAX_FILE_SIZE) {

757 + warning(_("ignoring overly large gitattributes blob '%s'"), path);

758 + return NULL;

759 + }

760

761 res = xcalloc(1, sizeof(*res));

762 for (sp = buf; *sp; ) {

attr.h

@@ -113,6 +113,12 @@

113 */

114 #define ATTR_MAX_LINE_LENGTH 2048

115

116 + /**

117 + * The maximum size of the giattributes file. If the file exceeds this size we

118 + * will ignore it.

119 + */

120 +#define ATTR_MAX_FILE_SIZE (100 * 1024 * 1024)

121 +

122 struct index_state;

123

124 /**

t/t0003-attributes.sh

+17

@@ -361,6 +361,14 @@ test_expect_success 'large attributes line ignores trailing content in tree' '

361 test_must_be_empty actual

362 '

363

364 +test_expect_success EXPENSIVE 'large attributes file ignored in tree' '

365 + test_when_finished "rm .gitattributes" &&

366 + dd if=/dev/zero of=.gitattributes bs=101M count=1 2>/dev/null &&

367 + git check-attr --all path >/dev/null 2>err &&

368 + echo "warning: ignoring overly large gitattributes file ${SQ}.gitattributes${SQ}" >expect &&

369 + test_cmp expect err

370 +'

371 +

372 test_expect_success 'large attributes line ignored in index' '

373 test_when_finished "git update-index --remove .gitattributes" &&

374 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' '

389 test_must_be_empty actual

390 '

391

392 +test_expect_success EXPENSIVE 'large attributes file ignored in index' '

393 + test_when_finished "git update-index --remove .gitattributes" &&

394 + blob=$(dd if=/dev/zero bs=101M count=1 2>/dev/null | git hash-object -w --stdin) &&

395 + git update-index --add --cacheinfo 100644,$blob,.gitattributes &&

396 + git check-attr --cached --all path >/dev/null 2>err &&

397 + echo "warning: ignoring overly large gitattributes blob ${SQ}.gitattributes${SQ}" >expect &&

398 + test_cmp expect err

399 +'

400 +

401 test_done