So, this re-introduces the bug addressed by commit 05bab3ea. The test number
is now 29 (rather than 21) but the same test is failing; namely
t3200-branch.sh
test #29 (git branch -m q q2 without config should succeed).
In order to fix the bug, I created the patch given below (on top of
this patch).
(Note that it does not address the above issues).
HTH
ATB,
Ramsay Jones
-- >8 --
From: Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxx>
Subject: [PATCH] path.c: Fix a static buffer overwrite bug by
avoiding mkpath()
The v4 version of the "Read (but not write) from XDG configuration,
XDG attributes and XDG ignore files" patch, re-introduced the bug
addressed by commit 05bab3ea ("config.c: Fix a static buffer overwrite
bug by avoiding mkpath()", 19-11-2011). Note that the patch refactored
the code to determine the user (or home) configuration filename into
a new function (home_config_paths()). In doing so, the new code once
again uses mkpath() rather than mksnpath().
In order to fix the bug, we introduce a new variation of the mkpath()
function, mkpathdup(), which avoids the use of the internal static
buffers. As the name implies, the new function returns a pointer to
the pathname as a dynamically allocated string. It is the callers
responsibility to free the memory for the returned string.
Having introduced the new function, we can now replace the calls to
'xstrdup(mkpath(...))' in the home_config_paths() function with a
call to mkpathdup() to achieve the same effect, without tickling the
original bug.
(Also, note that the 'xstrdup(mkpath(...))' pattern occurs in several
other places in the source ...)
Signed-off-by: Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxx>
---
cache.h | 2 ++
path.c | 20 +++++++++++++++++---
2 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/cache.h b/cache.h
index 0632503..fbba2d6 100644
--- a/cache.h
+++ b/cache.h
@@ -619,6 +619,8 @@ extern char *git_snpath(char *buf, size_t n,
const char *fmt, ...)
__attribute__((format (printf, 3, 4)));
extern char *git_pathdup(const char *fmt, ...)
__attribute__((format (printf, 1, 2)));
+extern char *mkpathdup(const char *fmt, ...)
+ __attribute__((format (printf, 1, 2)));
/* Return a statically allocated filename matching the sha1 signature */
extern char *mkpath(const char *fmt, ...) __attribute__((format
(printf, 1, 2)));
diff --git a/path.c b/path.c
index 53f3f53..ca29bdd 100644
--- a/path.c
+++ b/path.c
@@ -87,6 +87,20 @@ char *git_pathdup(const char *fmt, ...)
return xstrdup(path);
}
+char *mkpathdup(const char *fmt, ...)
+{
+ char path[PATH_MAX];
+ va_list args;
+ unsigned len;
+
+ va_start(args, fmt);
+ len = vsnprintf(path, sizeof(path), fmt, args);
+ va_end(args);
+ if (len >= sizeof(path))
+ return xstrdup(bad_path);
+ return xstrdup(cleanup_path(path));
+}
+
char *mkpath(const char *fmt, ...)
{
va_list args;
@@ -133,17 +147,17 @@ void home_config_paths(char **global, char
**xdg, char *file)
*global = NULL;
} else {
if (!xdg_home) {
- to_free = strdup(mkpath("%s/.config", home));
+ to_free = mkpathdup("%s/.config", home);
xdg_home = to_free;
}
if (global)
- *global = xstrdup(mkpath("%s/.gitconfig", home));
+ *global = mkpathdup("%s/.gitconfig", home);
}
if (!xdg_home)
*xdg = NULL;
else
- *xdg = xstrdup(mkpath("%s/git/%s", xdg_home, file));
+ *xdg = mkpathdup("%s/git/%s", xdg_home, file);
free(to_free);
}
--
1.7.10