Skip to content

Commit 97410b2

Browse files
drafnelgitster
authored andcommitted
attr.c: avoid inappropriate access to strbuf "buf" member
This code sequence performs a strcpy into the buf member of a strbuf struct. The strcpy may move the position of the terminating nul of the string and effectively change the length of string so that it does not match the len member of the strbuf struct. Currently, this sequence works since the strbuf was given a hint when it was initialized to allocate enough space to accomodate the string that will be strcpy'ed, but this is an implementation detail of strbufs, not a guarantee. So, lets rework this sequence so that the strbuf is only manipulated by strbuf functions, and direct modification of its "buf" member is not necessary. Signed-off-by: Brandon Casey <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 5738c9c commit 97410b2

File tree

1 file changed

+11
-13
lines changed

1 file changed

+11
-13
lines changed

attr.c

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -552,7 +552,6 @@ static void prepare_attr_stack(const char *path)
552552
{
553553
struct attr_stack *elem, *info;
554554
int dirlen, len;
555-
struct strbuf pathbuf;
556555
const char *cp;
557556

558557
cp = strrchr(path, '/');
@@ -561,8 +560,6 @@ static void prepare_attr_stack(const char *path)
561560
else
562561
dirlen = cp - path;
563562

564-
strbuf_init(&pathbuf, dirlen+2+strlen(GITATTRIBUTES_FILE));
565-
566563
/*
567564
* At the bottom of the attribute stack is the built-in
568565
* set of attribute definitions, followed by the contents
@@ -607,27 +604,28 @@ static void prepare_attr_stack(const char *path)
607604
* Read from parent directories and push them down
608605
*/
609606
if (!is_bare_repository() || direction == GIT_ATTR_INDEX) {
610-
while (1) {
611-
char *cp;
607+
struct strbuf pathbuf = STRBUF_INIT;
612608

609+
while (1) {
613610
len = strlen(attr_stack->origin);
614611
if (dirlen <= len)
615612
break;
616-
strbuf_reset(&pathbuf);
617-
strbuf_add(&pathbuf, path, dirlen);
613+
cp = memchr(path + len + 1, '/', dirlen - len - 1);
614+
if (!cp)
615+
cp = path + dirlen;
616+
strbuf_add(&pathbuf, path, cp - path);
618617
strbuf_addch(&pathbuf, '/');
619-
cp = strchr(pathbuf.buf + len + 1, '/');
620-
strcpy(cp + 1, GITATTRIBUTES_FILE);
618+
strbuf_addstr(&pathbuf, GITATTRIBUTES_FILE);
621619
elem = read_attr(pathbuf.buf, 0);
622-
*cp = '\0';
623-
elem->origin = strdup(pathbuf.buf);
620+
strbuf_setlen(&pathbuf, cp - path);
621+
elem->origin = strbuf_detach(&pathbuf, NULL);
624622
elem->prev = attr_stack;
625623
attr_stack = elem;
626624
debug_push(elem);
627625
}
628-
}
629626

630-
strbuf_release(&pathbuf);
627+
strbuf_release(&pathbuf);
628+
}
631629

632630
/*
633631
* Finally push the "info" one at the top of the stack.

0 commit comments

Comments
 (0)