Re: [PATCH] link_alt_odb_entry: fix read over array bounds reported by valgrind
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:54:22
Possibly related (same subject, not in this thread)
- 2016-06-15 · [PATCH] link_alt_odb_entry: fix read over array bounds reported by valgrind · Heiko Voigt <hidden>
- 2016-06-15 · Re: [PATCH] link_alt_odb_entry: fix read over array bounds reported by valgrind · Junio C Hamano <hidden>
Heiko Voigt [off-list ref] writes:
pfxlen can be longer than the path in objdir when relative_base contains the path to gits object directory.
s/gits/????/ perhaps "Git's", but I am not sure.
quoted hunk
Signed-off-by: Heiko Voigt <redacted> --- sha1_file.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)diff --git a/sha1_file.c b/sha1_file.c index 4ccaf7a..631d0dd 100644 --- a/sha1_file.c +++ b/sha1_file.c@@ -251,7 +251,7 @@ static int link_alt_odb_entry(const char * entry, int len, const char * relative const char *objdir = get_object_directory(); struct alternate_object_database *ent; struct alternate_object_database *alt; - int pfxlen, entlen; + int pfxlen, entlen, objdirlen; struct strbuf pathbuf = STRBUF_INIT; if (!is_absolute_path(entry) && relative_base) {@@ -298,7 +298,8 @@ static int link_alt_odb_entry(const char * entry, int len, const char * relative return -1; } } - if (!memcmp(ent->base, objdir, pfxlen)) { + objdirlen = strlen(objdir); + if (!memcmp(ent->base, objdir, pfxlen > objdirlen ? objdirlen : pfxlen)) {
The new code tells us to compare up to the shorter length between objdir (i.e. path/to/.git/objects) and the given alternate object directory (i.e. alt/path/to/.git/objects), but is that really what we want? What happens if the given alternate object directory were "path/to/.git/objects-not-quite", with objdir "path/to/.git/objects"? They are not the same directory, and this check is about avoiding "the common mistake of listing ... object directory itself", no?
free(ent); return -1; }