git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Subject: [PATCH 3/6] tree-walk: use size_t consistently
Date: Wed, 31 Jul 2019 00:38:18 -0400	[thread overview]
Message-ID: <20190731043818.GC27170@sigill.intra.peff.net> (raw)
In-Reply-To: <20190731043659.GA27028@sigill.intra.peff.net>

We store and manipulate the cumulative traverse_info.pathlen as an
"int", which can overflow when we are fed ridiculously long pathnames
(e.g., ones at the edge of 2GB or 4GB, even if the individual tree entry
names are smaller than that). The results can be confusing, though
after some prodding I was not able to use this integer overflow to cause
an under-allocated buffer.

Let's consistently use size_t to generarate and store these, and make
sure our addition doesn't overflow.

Signed-off-by: Jeff King <peff@peff.net>
---
 tree-walk.c    | 4 ++--
 tree-walk.h    | 6 +++---
 unpack-trees.c | 6 +++---
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/tree-walk.c b/tree-walk.c
index 130d9f32f2..cf5af6a46b 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -170,7 +170,7 @@ int tree_entry_gently(struct tree_desc *desc, struct name_entry *entry)
 
 void setup_traverse_info(struct traverse_info *info, const char *base)
 {
-	int pathlen = strlen(base);
+	size_t pathlen = strlen(base);
 	static struct traverse_info dummy;
 
 	memset(info, 0, sizeof(*info));
@@ -186,7 +186,7 @@ void setup_traverse_info(struct traverse_info *info, const char *base)
 char *make_traverse_path(char *path, const struct traverse_info *info,
 			 const char *name, size_t namelen)
 {
-	int pathlen = info->pathlen;
+	size_t pathlen = info->pathlen;
 
 	path[pathlen + namelen] = 0;
 	for (;;) {
diff --git a/tree-walk.h b/tree-walk.h
index 2c59caa38a..95de0506c8 100644
--- a/tree-walk.h
+++ b/tree-walk.h
@@ -62,7 +62,7 @@ struct traverse_info {
 	size_t namelen;
 	unsigned mode;
 
-	int pathlen;
+	size_t pathlen;
 	struct pathspec *pathspec;
 
 	unsigned long df_conflicts;
@@ -76,9 +76,9 @@ char *make_traverse_path(char *path, const struct traverse_info *info,
 			 const char *name, size_t namelen);
 void setup_traverse_info(struct traverse_info *info, const char *base);
 
-static inline int traverse_path_len(const struct traverse_info *info, const struct name_entry *n)
+static inline size_t traverse_path_len(const struct traverse_info *info, const struct name_entry *n)
 {
-	return info->pathlen + tree_entry_len(n);
+	return st_add(info->pathlen, tree_entry_len(n));
 }
 
 /* in general, positive means "kind of interesting" */
diff --git a/unpack-trees.c b/unpack-trees.c
index 63cbddead8..a014ae9907 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -686,7 +686,7 @@ static int index_pos_by_traverse_info(struct name_entry *names,
 				      struct traverse_info *info)
 {
 	struct unpack_trees_options *o = info->data;
-	int len = traverse_path_len(info, names);
+	size_t len = traverse_path_len(info, names);
 	char *name = xmalloc(len + 1 /* slash */ + 1 /* NUL */);
 	int pos;
 
@@ -814,7 +814,7 @@ static int traverse_trees_recursive(int n, unsigned long dirmask,
 	newinfo.name = p->path;
 	newinfo.namelen = p->pathlen;
 	newinfo.mode = p->mode;
-	newinfo.pathlen += tree_entry_len(p) + 1;
+	newinfo.pathlen = st_add3(newinfo.pathlen, tree_entry_len(p), 1);
 	newinfo.df_conflicts |= df_conflicts;
 
 	/*
@@ -960,7 +960,7 @@ static struct cache_entry *create_ce_entry(const struct traverse_info *info,
 	struct index_state *istate,
 	int is_transient)
 {
-	int len = traverse_path_len(info, n);
+	size_t len = traverse_path_len(info, n);
 	struct cache_entry *ce =
 		is_transient ?
 		make_empty_transient_cache_entry(len) :
-- 
2.23.0.rc0.426.gbdee707ba7


  parent reply	other threads:[~2019-07-31  4:38 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-31  4:37 [PATCH 0/6] harden tree-walking against integer overflow Jeff King
2019-07-31  4:38 ` [PATCH 1/6] setup_traverse_info(): stop copying oid Jeff King
2019-07-31  4:38 ` [PATCH 2/6] tree-walk: drop oid from traverse_info Jeff King
2019-07-31  4:38 ` Jeff King [this message]
2019-08-01 18:17   ` [PATCH 3/6] tree-walk: use size_t consistently Derrick Stolee
2019-07-31  4:38 ` [PATCH 4/6] tree-walk: accept a raw length for traverse_path_len() Jeff King
2019-07-31  4:38 ` [PATCH 5/6] tree-walk: add a strbuf wrapper for make_traverse_path() Jeff King
2019-07-31  4:38 ` [PATCH 6/6] tree-walk: harden make_traverse_path() length computations Jeff King

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20190731043818.GC27170@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).