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>

diff --git a/attr.c b/attr.c
index 679e42258c2b205bfceb9d065295c47c375442d4..7c380c173171e98d24a8d9d17125f1e09871497c 100644 (file)
--- a/attr.c
+++ b/attr.c
@@ -765,8 +765,8 @@ static struct attr_stack *read_attr_from_file(const char *path, unsigned flags)
        return res;
 }
 
-static struct attr_stack *read_attr_from_buf(char *buf, const char *path,
-                                            unsigned flags)
+static struct attr_stack *read_attr_from_buf(char *buf, size_t length,
+                                            const char *path, unsigned flags)
 {
        struct attr_stack *res;
        char *sp;
@@ -774,6 +774,11 @@ static struct attr_stack *read_attr_from_buf(char *buf, const char *path,
 
        if (!buf)
                return NULL;
+       if (length >= ATTR_MAX_FILE_SIZE) {
+               warning(_("ignoring overly large gitattributes blob '%s'"), path);
+               free(buf);
+               return NULL;
+       }
 
        CALLOC_ARRAY(res, 1);
        for (sp = buf; *sp;) {
@@ -813,7 +818,7 @@ static struct attr_stack *read_attr_from_blob(struct index_state *istate,
                return NULL;
        }
 
-       return read_attr_from_buf(buf, path, flags);
+       return read_attr_from_buf(buf, sz, path, flags);
 }
 
 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,
                stack = read_attr_from_blob(istate, &istate->cache[sparse_dir_pos]->oid, relative_path, flags);
        } else {
                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;
-               }
-               stack = read_attr_from_buf(buf, path, flags);
+               stack = read_attr_from_buf(buf, size, path, flags);
        }
        return stack;
 }

diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index 774b52c2980ca10e6183cf74ad10427f00c2514b..b007f76fd6b9b7298eb37d1a0d6a3f7d984b7ffe 100755 (executable)
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -572,6 +572,16 @@ test_expect_success EXPENSIVE 'large attributes file ignored in index' '
        test_cmp expect err
 '
 
+test_expect_success EXPENSIVE 'large attributes blob ignored' '
+       test_when_finished "git update-index --remove .gitattributes" &&
+       blob=$(dd if=/dev/zero bs=1048576 count=101 2>/dev/null | git hash-object -w --stdin) &&
+       git update-index --add --cacheinfo 100644,$blob,.gitattributes &&
+       tree="$(git write-tree)" &&
+       git check-attr --cached --all --source="$tree" path >/dev/null 2>err &&
+       echo "warning: ignoring overly large gitattributes blob ${SQ}.gitattributes${SQ}" >expect &&
+       test_cmp expect err
+'
+
 test_expect_success 'builtin object mode attributes work (dir and regular paths)' '
        >normal &&
        attr_check_object_mode normal 100644 &&