[PATCH v3 1/5] attr.c: avoid inappropriate access to strbuf "buf" member
- From
Brandon Casey <casey@nrlssc.navy.mil>
- Date
- Oct 6, 2011, 18:22 UTC
- Message-ID
- <U4wiHVyDLLG1PhI-8iY3YpRCHWWi1IE65h4oZsMtte9f21i1djIDHGQtHssxat_eXec7H3Qbo4MfDoWvguInipQUJgljvIPZw-HcavAqSnyX27VBdUvY8BvWiIBaYW193fZG_SNtSjg@cipher.nrlssc.navy.mil>
- In-Reply-To
- <VYN8m1JCy102-eaWWa-bsunEvt3zeXLJkVg7FZKZCtXT-Ww0vg7a8xA7NTvrZTiovKTnJ9Hlom0@cipher.nrlssc.navy.mil>
From: Brandon Casey <drafnel@gmail.com>
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> --- attr.c | 24 +++++++++++------------- 1 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/attr.c b/attr.c index 33cb4e4..fe38fcc 100644 --- a/attr.c +++ b/attr.c @@ -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, '/'); @@ -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 @@ -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.
-- 1.7.7