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 <drafnel@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Brandon Casey committed Oct 6, 2011 at 13:22 UTC 97410b27e91593c4096bb16abbace783895a1f62
1 file changed +11 -13
attr.c
+11 -13
@@ -552,7 +552,6 @@ static void prepare_attr_stack(const char *path)
552 {
553 struct attr_stack *elem, *info;
554 int dirlen, len;
555 - struct strbuf pathbuf;
555 const char *cp;
556
557 cp = strrchr(path, '/');
@@ -561,8 +560,6 @@ static void prepare_attr_stack(const char *path)
560 else
561 dirlen = cp - path;
562
564 - strbuf_init(&pathbuf, dirlen+2+strlen(GITATTRIBUTES_FILE));
565 -
563 /*
564 * At the bottom of the attribute stack is the built-in
565 * set of attribute definitions, followed by the contents
@@ -607,27 +604,28 @@ static void prepare_attr_stack(const char *path)
604 * Read from parent directories and push them down
605 */
606 if (!is_bare_repository() || direction == GIT_ATTR_INDEX) {
610 - while (1) {
611 - char *cp;
607 + struct strbuf pathbuf = STRBUF_INIT;
608
609 + while (1) {
610 len = strlen(attr_stack->origin);
611 if (dirlen <= len)
612 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);
617 strbuf_addch(&pathbuf, '/');
619 - cp = strchr(pathbuf.buf + len + 1, '/');
620 - strcpy(cp + 1, GITATTRIBUTES_FILE);
618 + strbuf_addstr(&pathbuf, GITATTRIBUTES_FILE);
619 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);
622 elem->prev = attr_stack;
623 attr_stack = elem;
624 debug_push(elem);
625 }
628 - }
626
630 - strbuf_release(&pathbuf);
627 + strbuf_release(&pathbuf);
628 + }
629
630 /*
631 * Finally push the "info" one at the top of the stack.