Skip to content

Commit

Permalink
attr.c: avoid inappropriate access to strbuf "buf" member
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
drafnel authored and gitster committed Oct 6, 2011
1 parent 5738c9c commit 97410b2
Showing 1 changed file with 11 additions and 13 deletions.
24 changes: 11 additions & 13 deletions attr.c
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,6 @@ static void prepare_attr_stack(const char *path)
{
struct attr_stack *elem, *info;
int dirlen, len;
struct strbuf pathbuf;
const char *cp;

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

strbuf_init(&pathbuf, dirlen+2+strlen(GITATTRIBUTES_FILE));

/*
* At the bottom of the attribute stack is the built-in
* set of attribute definitions, followed by the contents
Expand Down Expand Up @@ -607,27 +604,28 @@ static void prepare_attr_stack(const char *path)
* Read from parent directories and push them down
*/
if (!is_bare_repository() || direction == GIT_ATTR_INDEX) {
while (1) {
char *cp;
struct strbuf pathbuf = STRBUF_INIT;

while (1) {
len = strlen(attr_stack->origin);
if (dirlen <= len)
break;
strbuf_reset(&pathbuf);
strbuf_add(&pathbuf, path, dirlen);
cp = memchr(path + len + 1, '/', dirlen - len - 1);
if (!cp)
cp = path + dirlen;
strbuf_add(&pathbuf, path, cp - path);
strbuf_addch(&pathbuf, '/');
cp = strchr(pathbuf.buf + len + 1, '/');
strcpy(cp + 1, GITATTRIBUTES_FILE);
strbuf_addstr(&pathbuf, GITATTRIBUTES_FILE);
elem = read_attr(pathbuf.buf, 0);
*cp = '\0';
elem->origin = strdup(pathbuf.buf);
strbuf_setlen(&pathbuf, cp - path);
elem->origin = strbuf_detach(&pathbuf, NULL);
elem->prev = attr_stack;
attr_stack = elem;
debug_push(elem);
}
}

strbuf_release(&pathbuf);
strbuf_release(&pathbuf);
}

/*
* Finally push the "info" one at the top of the stack.
Expand Down

0 comments on commit 97410b2

Please sign in to comment.