Re: [PATCH 2/3] config.c: Fix a static buffer overwrite bug by avoiding mkpath()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 21, 2011, 03:32 UTC
- Message-ID
- <7v4nxyi219.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <4EC80688.609@ramsay1.demon.co.uk>
Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
> In order to fix the problem, we replace the call to mkpath() with > a call to mksnpath() and provide our own buffer.
A longer term fix might be to get rid of unwanted git_config_* call from stat() callchain, but as a short-term fix, this patch with minor tweaks should do.
Show 16 quoted lines
> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> ---
> config.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/config.c b/config.c
> index edf9914..5a9ca84 100644
> --- a/config.c
> +++ b/config.c
> @@ -851,6 +851,7 @@ int git_config_system(void)
>
> int git_config_early(config_fn_t fn, void *data, const char *repo_config)
> {
> + char buf[4096];
> int ret = 0, found = 0;
> const char *home = NULL;Two points. (1) This buffer does not need to be inside the whole function scope, no? (2) s/4096/PATH_MAX/ to match what get_pathname() has been returning to the caller?
That is, something like the attached squashed in?
Show 14 quoted lines
> @@ -865,12 +866,11 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
>
> home = getenv("HOME");
> if (home) {
> - char *user_config = xstrdup(mkpath("%s/.gitconfig", home));
> + char *user_config = mksnpath(buf, sizeof(buf), "%s/.gitconfig", home);
> if (!access(user_config, R_OK)) {
> ret += git_config_from_file(fn, user_config, data);
> found += 1;
> }
> - free(user_config);
> }
>
> if (repo_config && !access(repo_config, R_OK)) {config.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/config.c b/config.c index 5a9ca84..b6d789a 100644 --- a/config.c +++ b/config.c @@ -851,7 +851,6 @@ int git_config_system(void) int git_config_early(config_fn_t fn, void *data, const char *repo_config) { - char buf[4096]; int ret = 0, found = 0; const char *home = NULL; @@ -866,6 +865,7 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config) home = getenv("HOME"); if (home) { + char buf[PATH_MAX]; char *user_config = mksnpath(buf, sizeof(buf), "%s/.gitconfig", home); if (!access(user_config, R_OK)) { ret += git_config_from_file(fn, user_config, data);