From fd356f6aa8bb75ebef56fbc61caf7e02517fa6d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Thu, 24 Oct 2013 08:55:35 +0700 Subject: entry.c: convert checkout_entry to use strbuf MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old code does not do boundary check so any paths longer than PATH_MAX can cause buffer overflow. Replace it with strbuf to handle paths of arbitrary length. The OS may reject if the path is too long though. But in that case we report the cause (e.g. name too long) and usually move on to checking out the next entry. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- entry.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/entry.c b/entry.c index acc892f9a..fbb486310 100644 --- a/entry.c +++ b/entry.c @@ -237,16 +237,19 @@ static int check_path(const char *path, int len, struct stat *st, int skiplen) int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *topath) { - static char path[PATH_MAX + 1]; + static struct strbuf path_buf = STRBUF_INIT; + char *path; struct stat st; - int len = state->base_dir_len; + int len; if (topath) return write_entry(ce, topath, state, 1); - memcpy(path, state->base_dir, len); - strcpy(path + len, ce->name); - len += ce_namelen(ce); + strbuf_reset(&path_buf); + strbuf_add(&path_buf, state->base_dir, state->base_dir_len); + strbuf_add(&path_buf, ce->name, ce_namelen(ce)); + path = path_buf.buf; + len = path_buf.len; if (!check_path(path, len, &st, state->base_dir_len)) { unsigned changed = ce_match_stat(ce, &st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE); -- cgit v1.2.1 From af2a651d2ecb3967244c9a44a5813a0cf9977df5 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 23 Oct 2013 10:52:42 -0700 Subject: checkout_entry(): clarify the use of topath[] parameter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The said function has this signature: extern int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *topath); At first glance, it might appear that the caller of checkout_entry() can specify to which path the contents are written out by the last parameter, and it is tempting to add "const" in front of its type. In reality, however, topath[] is to point at a buffer to store the temporary path generated by the callchain originating from this function, and the temporary path is always short, much shorter than the buffer prepared by its only caller in builtin/checkout-index.c. Document the code a bit to clarify so that future callers know how to use the function better. Noticed-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/checkout-index.c | 2 +- cache.h | 1 + entry.c | 8 ++++++++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c index b1feda7d5..4ed6b2395 100644 --- a/builtin/checkout-index.c +++ b/builtin/checkout-index.c @@ -14,7 +14,7 @@ static int line_termination = '\n'; static int checkout_stage; /* default to checkout stage0 */ static int to_tempfile; -static char topath[4][PATH_MAX + 1]; +static char topath[4][TEMPORARY_FILENAME_LENGTH + 1]; static struct checkout state; diff --git a/cache.h b/cache.h index 85b544f38..3118b7fc9 100644 --- a/cache.h +++ b/cache.h @@ -975,6 +975,7 @@ struct checkout { refresh_cache:1; }; +#define TEMPORARY_FILENAME_LENGTH 25 extern int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *topath); struct cache_def { diff --git a/entry.c b/entry.c index fbb486310..7b7aa8167 100644 --- a/entry.c +++ b/entry.c @@ -234,6 +234,14 @@ static int check_path(const char *path, int len, struct stat *st, int skiplen) return lstat(path, st); } +/* + * Write the contents from ce out to the working tree. + * + * When topath[] is not NULL, instead of writing to the working tree + * file named by ce, a temporary file is created by this function and + * its name is returned in topath[], which must be able to hold at + * least TEMPORARY_FILENAME_LENGTH bytes long. + */ int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *topath) { -- cgit v1.2.1