diff options
author | Patrick Steinhardt <ps@pks.im> | 2022-12-01 15:45:53 +0100 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2022-12-05 07:50:03 +0100 |
commit | 3c50032ff5289cc45659f21949c8d09e52164579 (patch) | |
tree | 2d9a9379e7d94be5d8ad773466c90a76b79034d5 | |
parent | attr: ignore attribute lines exceeding 2048 bytes (diff) | |
download | git-3c50032ff5289cc45659f21949c8d09e52164579.tar.xz git-3c50032ff5289cc45659f21949c8d09e52164579.zip |
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>
-rw-r--r-- | attr.c | 24 | ||||
-rw-r--r-- | attr.h | 6 | ||||
-rwxr-xr-x | t/t0003-attributes.sh | 17 |
3 files changed, 45 insertions, 2 deletions
@@ -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; ) { @@ -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 7d68e6a56e..9d9aa2855d 100755 --- 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 |