@matheusbsbs / matheus-bagliano / commits / c793f9cb08

attr.c: move ATTR_MAX_FILE_SIZE check into read_attr_from_buf()

Commit 3c50032ff52 (attr: ignore overly large gitattributes files, 2022-12-01) added a defense-in-depth check to ensure that .gitattributes blobs read from the index do not exceed ATTR_MAX_FILE_SIZE (100 MB). But there were two cases added shortly after 3c50032ff52 was written which do not apply similar protections: - 47cfc9bd7d0 (attr: add flag `--source` to work with tree-ish, 2023-01-14) - 4723ae1007f (attr.c: read attributes in a sparse directory, 2023-08-11) added a similar Ensure that we refuse to process a .gitattributes blob exceeding ATTR_MAX_FILE_SIZE when reading from either an arbitrary tree object or a sparse directory. This is done by pushing the ATTR_MAX_FILE_SIZE check down into the low-level `read_attr_from_buf()`. In doing so, plug a leak in `read_attr_from_index()` where we would accidentally leak the large buffer upon detecting it is too large to process. (Since `read_attr_from_buf()` handles a NULL buffer input, we can remove a NULL check before calling it in `read_attr_from_index()` as well). Co-authored-by: Jeff King <peff@peff.net> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed May 3, 2024 at 15:12 UTC c793f9cb0853b7b173228efa53b32c60e3818598

2 files changed +19 -10

attr.c

+9 -10

@@ -765,8 +765,8 @@ static struct attr_stack *read_attr_from_file(const char *path, unsigned flags)

765 return res;

766 }

767

768 -static struct attr_stack *read_attr_from_buf(char *buf, const char *path,

769 - unsigned flags)

768 +static struct attr_stack *read_attr_from_buf(char *buf, size_t length,

769 + const char *path, unsigned flags)

770 {

771 struct attr_stack *res;

772 char *sp;

@@ -774,6 +774,11 @@ static struct attr_stack *read_attr_from_buf(char *buf, const char *path,

774

775 if (!buf)

776 return NULL;

777 + if (length >= ATTR_MAX_FILE_SIZE) {

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

779 + free(buf);

780 + return NULL;

781 + }

782

783 CALLOC_ARRAY(res, 1);

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

@@ -813,7 +818,7 @@ static struct attr_stack *read_attr_from_blob(struct index_state *istate,

818 return NULL;

819 }

820

816 - return read_attr_from_buf(buf, path, flags);

821 + return read_attr_from_buf(buf, sz, path, flags);

822 }

823

824 static struct attr_stack *read_attr_from_index(struct index_state *istate,

@@ -860,13 +865,7 @@ static struct attr_stack *read_attr_from_index(struct index_state *istate,

865 stack = read_attr_from_blob(istate, &istate->cache[sparse_dir_pos]->oid, relative_path, flags);

866 } else {

867 buf = read_blob_data_from_index(istate, path, &size);

863 - if (!buf)

864 - return NULL;

865 - if (size >= ATTR_MAX_FILE_SIZE) {

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

867 - return NULL;

868 - }

869 - stack = read_attr_from_buf(buf, path, flags);

868 + stack = read_attr_from_buf(buf, size, path, flags);

869 }

870 return stack;

871 }

t/t0003-attributes.sh

+10

@@ -572,6 +572,16 @@ test_expect_success EXPENSIVE 'large attributes file ignored in index' '

572 test_cmp expect err

573 '

574

575 +test_expect_success EXPENSIVE 'large attributes blob ignored' '

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

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

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

579 + tree="$(git write-tree)" &&

580 + git check-attr --cached --all --source="$tree" path >/dev/null 2>err &&

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

582 + test_cmp expect err

583 +'

584 +

585 test_expect_success 'builtin object mode attributes work (dir and regular paths)' '

586 >normal &&

587 attr_check_object_mode normal 100644 &&