attr: ignore attribute lines exceeding 2048 bytes

There are two different code paths to read gitattributes: once via a file, and once via the index. These two paths used to behave differently because when reading attributes from a file, we used fgets(3P) with a buffer size of 2kB. Consequentially, we silently truncate line lengths when lines are longer than that and will then parse the remainder of the line as a new pattern. It goes without saying that this is entirely unexpected, but it's even worse that the behaviour depends on how the gitattributes are parsed. While this is simply wrong, the silent truncation saves us with the recently discovered vulnerabilities that can cause out-of-bound writes or reads with unreasonably long lines due to integer overflows. As the common path is to read gitattributes via the worktree file instead of via the index, we can assume that any gitattributes file that had lines longer than that is already broken anyway. So instead of lifting the limit here, we can double down on it to fix the vulnerabilities. Introduce an explicit line length limit of 2kB that is shared across all paths that read attributes and ignore any line that hits this limit while printing a warning. 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 dfa6b32b5e599d97448337ed4fc18dd50c90758f
3 files changed +34 -2
attr.c
+5
@@ -344,6 +344,11 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
344 return NULL;
345 name = cp;
346
347 + if (strlen(line) >= ATTR_MAX_LINE_LENGTH) {
348 + warning(_("ignoring overly long attributes line %d"), lineno);
349 + return NULL;
350 + }
351 +
352 if (*cp == '"' && !unquote_c_style(&pattern, name, &states)) {
353 name = pattern.buf;
354 namelen = pattern.len;
attr.h
+6
@@ -107,6 +107,12 @@
107 * - Free the `attr_check` struct by calling `attr_check_free()`.
108 */
109
110 +/**
111 + * The maximum line length for a gitattributes file. If the line exceeds this
112 + * length we will ignore it.
113 + */
114 +#define ATTR_MAX_LINE_LENGTH 2048
115 +
116 struct index_state;
117
118 /**
t/t0003-attributes.sh
+23 -2
@@ -339,6 +339,15 @@ test_expect_success 'query binary macro directly' '
339 test_cmp expect actual
340 '
341
342 +test_expect_success 'large attributes line ignored in tree' '
343 + test_when_finished "rm .gitattributes" &&
344 + printf "path %02043d" 1 >.gitattributes &&
345 + git check-attr --all path >actual 2>err &&
346 + echo "warning: ignoring overly long attributes line 1" >expect &&
347 + test_cmp expect err &&
348 + test_must_be_empty actual
349 +'
350 +
351 test_expect_success 'large attributes line ignores trailing content in tree' '
352 test_when_finished "rm .gitattributes" &&
353 # older versions of Git broke lines at 2048 bytes; the 2045 bytes
@@ -347,7 +356,18 @@ test_expect_success 'large attributes line ignores trailing content in tree' '
356 # erroneously parsed.
357 printf "a %02045dtrailing attribute\n" 1 >.gitattributes &&
358 git check-attr --all trailing >actual 2>err &&
350 - test_must_be_empty err &&
359 + echo "warning: ignoring overly long attributes line 1" >expect &&
360 + test_cmp expect err &&
361 + test_must_be_empty actual
362 +'
363 +
364 +test_expect_success 'large attributes line ignored in index' '
365 + test_when_finished "git update-index --remove .gitattributes" &&
366 + blob=$(printf "path %02043d" 1 | git hash-object -w --stdin) &&
367 + git update-index --add --cacheinfo 100644,$blob,.gitattributes &&
368 + git check-attr --cached --all path >actual 2>err &&
369 + echo "warning: ignoring overly long attributes line 1" >expect &&
370 + test_cmp expect err &&
371 test_must_be_empty actual
372 '
373
@@ -356,7 +376,8 @@ test_expect_success 'large attributes line ignores trailing content in index' '
376 blob=$(printf "a %02045dtrailing attribute\n" 1 | git hash-object -w --stdin) &&
377 git update-index --add --cacheinfo 100644,$blob,.gitattributes &&
378 git check-attr --cached --all trailing >actual 2>err &&
359 - test_must_be_empty err &&
379 + echo "warning: ignoring overly long attributes line 1" >expect &&
380 + test_cmp expect err &&
381 test_must_be_empty actual
382 '
383