Skip to content

Commit e0bfc0b

Browse files
gitstergitster
committed
Merge branch 'ps/attr-limits' into maint-2.32
2 parents af778cd + 3c50032 commit e0bfc0b

File tree

3 files changed

+127
-35
lines changed

3 files changed

+127
-35
lines changed

‎attr.c

Lines changed: 56 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ static const char git_attr__unknown[] = "(builtin)unknown";
2828
#endif
2929

3030
struct git_attr {
31-
int attr_nr; /* unique attribute number */
31+
unsigned int attr_nr; /* unique attribute number */
3232
char name[FLEX_ARRAY]; /* attribute name */
3333
};
3434

@@ -210,7 +210,7 @@ static void report_invalid_attr(const char *name, size_t len,
210210
* dictionary. If no entry is found, create a new attribute and store it in
211211
* the dictionary.
212212
*/
213-
static const struct git_attr *git_attr_internal(const char *name, int namelen)
213+
static const struct git_attr *git_attr_internal(const char *name, size_t namelen)
214214
{
215215
struct git_attr *a;
216216

@@ -226,8 +226,8 @@ static const struct git_attr *git_attr_internal(const char *name, int namelen)
226226
a->attr_nr = hashmap_get_size(&g_attr_hashmap.map);
227227

228228
attr_hashmap_add(&g_attr_hashmap, a->name, namelen, a);
229-
assert(a->attr_nr ==
230-
(hashmap_get_size(&g_attr_hashmap.map) - 1));
229+
if (a->attr_nr != hashmap_get_size(&g_attr_hashmap.map) - 1)
230+
die(_("unable to add additional attribute"));
231231
}
232232

233233
hashmap_unlock(&g_attr_hashmap);
@@ -272,7 +272,7 @@ struct match_attr {
272272
const struct git_attr *attr;
273273
} u;
274274
char is_macro;
275-
unsigned num_attr;
275+
size_t num_attr;
276276
struct attr_state state[FLEX_ARRAY];
277277
};
278278

@@ -293,7 +293,7 @@ static const char *parse_attr(const char *src, int lineno, const char *cp,
293293
struct attr_state *e)
294294
{
295295
const char *ep, *equals;
296-
int len;
296+
size_t len;
297297

298298
ep = cp + strcspn(cp, blank);
299299
equals = strchr(cp, '=');
@@ -337,8 +337,7 @@ static const char *parse_attr(const char *src, int lineno, const char *cp,
337337
static struct match_attr *parse_attr_line(const char *line, const char *src,
338338
int lineno, unsigned flags)
339339
{
340-
int namelen;
341-
int num_attr, i;
340+
size_t namelen, num_attr, i;
342341
const char *cp, *name, *states;
343342
struct match_attr *res = NULL;
344343
int is_macro;
@@ -349,6 +348,11 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
349348
return NULL;
350349
name = cp;
351350

351+
if (strlen(line) >= ATTR_MAX_LINE_LENGTH) {
352+
warning(_("ignoring overly long attributes line %d"), lineno);
353+
return NULL;
354+
}
355+
352356
if (*cp == '"' && !unquote_c_style(&pattern, name, &states)) {
353357
name = pattern.buf;
354358
namelen = pattern.len;
@@ -385,10 +389,9 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
385389
goto fail_return;
386390
}
387391

388-
res = xcalloc(1,
389-
sizeof(*res) +
390-
sizeof(struct attr_state) * num_attr +
391-
(is_macro ? 0 : namelen + 1));
392+
res = xcalloc(1, st_add3(sizeof(*res),
393+
st_mult(sizeof(struct attr_state), num_attr),
394+
is_macro ? 0 : namelen + 1));
392395
if (is_macro) {
393396
res->u.attr = git_attr_internal(name, namelen);
394397
} else {
@@ -451,11 +454,12 @@ struct attr_stack {
451454

452455
static void attr_stack_free(struct attr_stack *e)
453456
{
454-
int i;
457+
unsigned i;
455458
free(e->origin);
456459
for (i = 0; i < e->num_matches; i++) {
457460
struct match_attr *a = e->attrs[i];
458-
int j;
461+
size_t j;
462+
459463
for (j = 0; j < a->num_attr; j++) {
460464
const char *setto = a->state[j].setto;
461465
if (setto == ATTR__TRUE ||
@@ -664,8 +668,8 @@ static void handle_attr_line(struct attr_stack *res,
664668
a = parse_attr_line(line, src, lineno, flags);
665669
if (!a)
666670
return;
667-
ALLOC_GROW(res->attrs, res->num_matches + 1, res->alloc);
668-
res->attrs[res->num_matches++] = a;
671+
ALLOC_GROW_BY(res->attrs, res->num_matches, 1, res->alloc);
672+
res->attrs[res->num_matches - 1] = a;
669673
}
670674

671675
static struct attr_stack *read_attr_from_array(const char **list)
@@ -705,11 +709,12 @@ void git_attr_set_direction(enum git_attr_direction new_direction)
705709

706710
static struct attr_stack *read_attr_from_file(const char *path, unsigned flags)
707711
{
712+
struct strbuf buf = STRBUF_INIT;
708713
int fd;
709714
FILE *fp;
710715
struct attr_stack *res;
711-
char buf[2048];
712716
int lineno = 0;
717+
struct stat st;
713718

714719
if (flags & READ_ATTR_NOFOLLOW)
715720
fd = open_nofollow(path, O_RDONLY);
@@ -721,15 +726,26 @@ static struct attr_stack *read_attr_from_file(const char *path, unsigned flags)
721726
return NULL;
722727
}
723728
fp = xfdopen(fd, "r");
729+
if (fstat(fd, &st)) {
730+
warning_errno(_("cannot fstat gitattributes file '%s'"), path);
731+
fclose(fp);
732+
return NULL;
733+
}
734+
if (st.st_size >= ATTR_MAX_FILE_SIZE) {
735+
warning(_("ignoring overly large gitattributes file '%s'"), path);
736+
fclose(fp);
737+
return NULL;
738+
}
724739

725740
CALLOC_ARRAY(res, 1);
726-
while (fgets(buf, sizeof(buf), fp)) {
727-
char *bufp = buf;
728-
if (!lineno)
729-
skip_utf8_bom(&bufp, strlen(bufp));
730-
handle_attr_line(res, bufp, path, ++lineno, flags);
741+
while (strbuf_getline(&buf, fp) != EOF) {
742+
if (!lineno && starts_with(buf.buf, utf8_bom))
743+
strbuf_remove(&buf, 0, strlen(utf8_bom));
744+
handle_attr_line(res, buf.buf, path, ++lineno, flags);
731745
}
746+
732747
fclose(fp);
748+
strbuf_release(&buf);
733749
return res;
734750
}
735751

@@ -740,13 +756,18 @@ static struct attr_stack *read_attr_from_index(struct index_state *istate,
740756
struct attr_stack *res;
741757
char *buf, *sp;
742758
int lineno = 0;
759+
size_t size;
743760

744761
if (!istate)
745762
return NULL;
746763

747-
buf = read_blob_data_from_index(istate, path, NULL);
764+
buf = read_blob_data_from_index(istate, path, &size);
748765
if (!buf)
749766
return NULL;
767+
if (size >= ATTR_MAX_FILE_SIZE) {
768+
warning(_("ignoring overly large gitattributes blob '%s'"), path);
769+
return NULL;
770+
}
750771

751772
CALLOC_ARRAY(res, 1);
752773
for (sp = buf; *sp; ) {
@@ -1017,12 +1038,12 @@ static int macroexpand_one(struct all_attrs_item *all_attrs, int nr, int rem);
10171038
static int fill_one(const char *what, struct all_attrs_item *all_attrs,
10181039
const struct match_attr *a, int rem)
10191040
{
1020-
int i;
1041+
size_t i;
10211042

1022-
for (i = a->num_attr - 1; rem > 0 && i >= 0; i--) {
1023-
const struct git_attr *attr = a->state[i].attr;
1043+
for (i = a->num_attr; rem > 0 && i > 0; i--) {
1044+
const struct git_attr *attr = a->state[i - 1].attr;
10241045
const char **n = &(all_attrs[attr->attr_nr].value);
1025-
const char *v = a->state[i].setto;
1046+
const char *v = a->state[i - 1].setto;
10261047

10271048
if (*n == ATTR__UNKNOWN) {
10281049
debug_set(what,
@@ -1041,11 +1062,11 @@ static int fill(const char *path, int pathlen, int basename_offset,
10411062
struct all_attrs_item *all_attrs, int rem)
10421063
{
10431064
for (; rem > 0 && stack; stack = stack->prev) {
1044-
int i;
1065+
unsigned i;
10451066
const char *base = stack->origin ? stack->origin : "";
10461067

1047-
for (i = stack->num_matches - 1; 0 < rem && 0 <= i; i--) {
1048-
const struct match_attr *a = stack->attrs[i];
1068+
for (i = stack->num_matches; 0 < rem && 0 < i; i--) {
1069+
const struct match_attr *a = stack->attrs[i - 1];
10491070
if (a->is_macro)
10501071
continue;
10511072
if (path_matches(path, pathlen, basename_offset,
@@ -1076,11 +1097,11 @@ static void determine_macros(struct all_attrs_item *all_attrs,
10761097
const struct attr_stack *stack)
10771098
{
10781099
for (; stack; stack = stack->prev) {
1079-
int i;
1080-
for (i = stack->num_matches - 1; i >= 0; i--) {
1081-
const struct match_attr *ma = stack->attrs[i];
1100+
unsigned i;
1101+
for (i = stack->num_matches; i > 0; i--) {
1102+
const struct match_attr *ma = stack->attrs[i - 1];
10821103
if (ma->is_macro) {
1083-
int n = ma->u.attr->attr_nr;
1104+
unsigned int n = ma->u.attr->attr_nr;
10841105
if (!all_attrs[n].macro) {
10851106
all_attrs[n].macro = ma;
10861107
}
@@ -1132,7 +1153,7 @@ void git_check_attr(struct index_state *istate,
11321153
collect_some_attrs(istate, path, check);
11331154

11341155
for (i = 0; i < check->nr; i++) {
1135-
size_t n = check->items[i].attr->attr_nr;
1156+
unsigned int n = check->items[i].attr->attr_nr;
11361157
const char *value = check->all_attrs[n].value;
11371158
if (value == ATTR__UNKNOWN)
11381159
value = ATTR__UNSET;

‎attr.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,18 @@
107107
* - Free the `attr_check` struct by calling `attr_check_free()`.
108108
*/
109109

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+
/**
117+
* The maximum size of the giattributes file. If the file exceeds this size we
118+
* will ignore it.
119+
*/
120+
#define ATTR_MAX_FILE_SIZE (100 * 1024 * 1024)
121+
110122
struct index_state;
111123

112124
/**

‎t/t0003-attributes.sh

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,4 +369,63 @@ test_expect_success SYMLINKS 'symlinks not respected in-tree' '
369369
test_i18ngrep "unable to access.*gitattributes" err
370370
'
371371

372+
test_expect_success 'large attributes line ignored in tree' '
373+
test_when_finished "rm .gitattributes" &&
374+
printf "path %02043d" 1 >.gitattributes &&
375+
git check-attr --all path >actual 2>err &&
376+
echo "warning: ignoring overly long attributes line 1" >expect &&
377+
test_cmp expect err &&
378+
test_must_be_empty actual
379+
'
380+
381+
test_expect_success 'large attributes line ignores trailing content in tree' '
382+
test_when_finished "rm .gitattributes" &&
383+
# older versions of Git broke lines at 2048 bytes; the 2045 bytes
384+
# of 0-padding here is accounting for the three bytes of "a 1", which
385+
# would knock "trailing" to the "next" line, where it would be
386+
# erroneously parsed.
387+
printf "a %02045dtrailing attribute\n" 1 >.gitattributes &&
388+
git check-attr --all trailing >actual 2>err &&
389+
echo "warning: ignoring overly long attributes line 1" >expect &&
390+
test_cmp expect err &&
391+
test_must_be_empty actual
392+
'
393+
394+
test_expect_success EXPENSIVE 'large attributes file ignored in tree' '
395+
test_when_finished "rm .gitattributes" &&
396+
dd if=/dev/zero of=.gitattributes bs=101M count=1 2>/dev/null &&
397+
git check-attr --all path >/dev/null 2>err &&
398+
echo "warning: ignoring overly large gitattributes file ${SQ}.gitattributes${SQ}" >expect &&
399+
test_cmp expect err
400+
'
401+
402+
test_expect_success 'large attributes line ignored in index' '
403+
test_when_finished "git update-index --remove .gitattributes" &&
404+
blob=$(printf "path %02043d" 1 | git hash-object -w --stdin) &&
405+
git update-index --add --cacheinfo 100644,$blob,.gitattributes &&
406+
git check-attr --cached --all path >actual 2>err &&
407+
echo "warning: ignoring overly long attributes line 1" >expect &&
408+
test_cmp expect err &&
409+
test_must_be_empty actual
410+
'
411+
412+
test_expect_success 'large attributes line ignores trailing content in index' '
413+
test_when_finished "git update-index --remove .gitattributes" &&
414+
blob=$(printf "a %02045dtrailing attribute\n" 1 | git hash-object -w --stdin) &&
415+
git update-index --add --cacheinfo 100644,$blob,.gitattributes &&
416+
git check-attr --cached --all trailing >actual 2>err &&
417+
echo "warning: ignoring overly long attributes line 1" >expect &&
418+
test_cmp expect err &&
419+
test_must_be_empty actual
420+
'
421+
422+
test_expect_success EXPENSIVE 'large attributes file ignored in index' '
423+
test_when_finished "git update-index --remove .gitattributes" &&
424+
blob=$(dd if=/dev/zero bs=101M count=1 2>/dev/null | git hash-object -w --stdin) &&
425+
git update-index --add --cacheinfo 100644,$blob,.gitattributes &&
426+
git check-attr --cached --all path >/dev/null 2>err &&
427+
echo "warning: ignoring overly large gitattributes blob ${SQ}.gitattributes${SQ}" >expect &&
428+
test_cmp expect err
429+
'
430+
372431
test_done

0 commit comments

Comments
 (0)