From: Taylor Blau <me@ttaylorr.com>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>, Patrick Steinhardt <ps@pks.im>,
Junio C Hamano <gitster@pobox.com>
Subject: [PATCH] attr.c: move ATTR_MAX_FILE_SIZE check into read_attr_from_buf()
Date: Fri, 3 May 2024 15:12:36 -0400 [thread overview]
Message-ID: <28f6267709db78ba526d7ed9fc4a734674697c70.1714763555.git.me@ttaylorr.com> (raw)
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>
---
attr.c | 19 +++++++++----------
t/t0003-attributes.sh | 10 ++++++++++
2 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/attr.c b/attr.c
index 679e42258c..7c380c1731 100644
--- 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 774b52c298..b007f76fd6 100755
--- 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 &&
--
2.45.0.36.g67f89f42283.dirty
next reply other threads:[~2024-05-03 19:12 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-03 19:12 Taylor Blau [this message]
2024-05-03 19:45 ` [PATCH] attr.c: move ATTR_MAX_FILE_SIZE check into read_attr_from_buf() Junio C Hamano
2024-05-06 5:57 ` Patrick Steinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=28f6267709db78ba526d7ed9fc4a734674697c70.1714763555.git.me@ttaylorr.com \
--to=me@ttaylorr.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
--cc=ps@pks.im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).