git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Brandon Casey <drafnel@gmail.com>
To: peff@peff.net
Cc: git@vger.kernel.org, gitster@pobox.com, sunshine@sunshineco.com,
	bharrosh@panasas.com, trast@student.ethz.ch, zapped@mail.ru,
	Brandon Casey <drafnel@gmail.com>
Subject: [PATCH 1/4] attr.c: avoid inappropriate access to strbuf "buf" member
Date: Wed, 14 Sep 2011 20:59:36 -0500	[thread overview]
Message-ID: <1316051979-19671-2-git-send-email-drafnel@gmail.com> (raw)
In-Reply-To: <1316051979-19671-1-git-send-email-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 |   25 ++++++++++++-------------
 1 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/attr.c b/attr.c
index 33cb4e4..206f233 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,29 @@ 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_add(&pathbuf, GITATTRIBUTES_FILE,
+				strlen(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.6.2

  reply	other threads:[~2011-09-15  2:00 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <5XXEFw0WjtXKd9dpXSxpkskCcgVyG9Db1_zzVSEBNey-kpXSBbmQfYaxZ2Szg6Pbck6hZZTQ5hHzBwG4rhKYXshrdmveEFLPZ9W0V8P_lw@cipher.nrlssc.navy.mil>
2011-09-15  1:59 ` [PATCH 0/4] Honor core.ignorecase for attribute patterns Brandon Casey
2011-09-15  1:59   ` Brandon Casey [this message]
2011-09-15  1:59   ` [PATCH 2/4] cleanup: use internal memory allocation wrapper functions everywhere Brandon Casey
2011-09-15  6:52     ` Johannes Sixt
2011-09-15 15:39       ` Brandon Casey
     [not found]         ` <CA+sFfMf73K3yv_5K633DKOsVufMV6rTjd+SSunq4sBikt4jCsg@mail.gmail.com>
2011-10-06  2:00           ` Brandon Casey
2011-10-06  6:17             ` Johannes Sixt
2011-10-06  7:01               ` Alexey Shumkin
2011-10-06 16:14               ` Brandon Casey
2011-10-06 16:50                 ` Erik Faye-Lund
2011-10-06 16:52                   ` Erik Faye-Lund
2011-10-06 17:17                   ` Brandon Casey
2011-09-15  1:59   ` [PATCH 3/4] builtin/mv.c: plug miniscule memory leak Brandon Casey
2011-09-15  1:59   ` [PATCH 4/4] attr.c: respect core.ignorecase when matching attribute patterns Brandon Casey
2011-09-15  4:01     ` Junio C Hamano
2011-09-15  4:06       ` Junio C Hamano
2011-09-15 15:38         ` Brandon Casey
2011-09-15 18:12   ` [PATCH 0/4] Honor core.ignorecase for " Jeff King
2011-09-15 20:28     ` Brandon Casey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1316051979-19671-2-git-send-email-drafnel@gmail.com \
    --to=drafnel@gmail.com \
    --cc=bharrosh@panasas.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    --cc=trast@student.ethz.ch \
    --cc=zapped@mail.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).