Skip to content

Commit

Permalink
Fix buffer overflow in git diff
Browse files Browse the repository at this point in the history 
If PATH_MAX on your system is smaller than a path stored, it may cause
buffer overflow and stack corruption in diff_addremove() and diff_change()
functions when running git-diff

Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
@dmpot @gitster
dmpot authored and gitster committed Jul 16, 2008
1 parent 620e2bb commit fd55a19
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 25 deletions.
8 changes: 4 additions & 4 deletions diff-lib.c
View file
Expand Up @@ -171,7 +171,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
if (silent_on_removed)
continue;
diff_addremove(&revs->diffopt, '-', ce->ce_mode,
ce->sha1, ce->name, NULL);
ce->sha1, ce->name);
continue;
}
changed = ce_match_stat(ce, &st, ce_option);
Expand All @@ -184,7 +184,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
newmode = ce_mode_from_stat(ce, st.st_mode);
diff_change(&revs->diffopt, oldmode, newmode,
ce->sha1, (changed ? null_sha1 : ce->sha1),
ce->name, NULL);
ce->name);

}
diffcore_std(&revs->diffopt);
Expand All @@ -208,7 +208,7 @@ static void diff_index_show_file(struct rev_info *revs,
const unsigned char *sha1, unsigned int mode)
{
diff_addremove(&revs->diffopt, prefix[0], mode,
sha1, ce->name, NULL);
sha1, ce->name);
}

static int get_stat_data(struct cache_entry *ce,
Expand Down Expand Up @@ -312,7 +312,7 @@ static int show_modified(struct oneway_unpack_data *cbdata,
return 0;

diff_change(&revs->diffopt, oldmode, mode,
old->sha1, sha1, old->name, NULL);
old->sha1, sha1, old->name);
return 0;
}

Expand Down
11 changes: 2 additions & 9 deletions diff.c
View file
Expand Up @@ -3356,9 +3356,8 @@ int diff_result_code(struct diff_options *opt, int status)
void diff_addremove(struct diff_options *options,
int addremove, unsigned mode,
const unsigned char *sha1,
const char *base, const char *path)
const char *concatpath)
{
char concatpath[PATH_MAX];
struct diff_filespec *one, *two;

if (DIFF_OPT_TST(options, IGNORE_SUBMODULES) && S_ISGITLINK(mode))
Expand All @@ -3380,9 +3379,6 @@ void diff_addremove(struct diff_options *options,
addremove = (addremove == '+' ? '-' :
addremove == '-' ? '+' : addremove);

if (!path) path = "";
sprintf(concatpath, "%s%s", base, path);

if (options->prefix &&
strncmp(concatpath, options->prefix, options->prefix_length))
return;
Expand All @@ -3403,9 +3399,8 @@ void diff_change(struct diff_options *options,
unsigned old_mode, unsigned new_mode,
const unsigned char *old_sha1,
const unsigned char *new_sha1,
const char *base, const char *path)
const char *concatpath)
{
char concatpath[PATH_MAX];
struct diff_filespec *one, *two;

if (DIFF_OPT_TST(options, IGNORE_SUBMODULES) && S_ISGITLINK(old_mode)
Expand All @@ -3418,8 +3413,6 @@ void diff_change(struct diff_options *options,
tmp = old_mode; old_mode = new_mode; new_mode = tmp;
tmp_c = old_sha1; old_sha1 = new_sha1; new_sha1 = tmp_c;
}
if (!path) path = "";
sprintf(concatpath, "%s%s", base, path);

if (options->prefix &&
strncmp(concatpath, options->prefix, options->prefix_length))
Expand Down
9 changes: 4 additions & 5 deletions diff.h
View file
Expand Up @@ -14,12 +14,12 @@ typedef void (*change_fn_t)(struct diff_options *options,
unsigned old_mode, unsigned new_mode,
const unsigned char *old_sha1,
const unsigned char *new_sha1,
const char *base, const char *path);
const char *fullpath);

typedef void (*add_remove_fn_t)(struct diff_options *options,
int addremove, unsigned mode,
const unsigned char *sha1,
const char *base, const char *path);
const char *fullpath);

typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
struct diff_options *options, void *data);
Expand Down Expand Up @@ -164,14 +164,13 @@ extern void diff_addremove(struct diff_options *,
int addremove,
unsigned mode,
const unsigned char *sha1,
const char *base,
const char *path);
const char *fullpath);

extern void diff_change(struct diff_options *,
unsigned mode1, unsigned mode2,
const unsigned char *sha1,
const unsigned char *sha2,
const char *base, const char *path);
const char *fullpath);

extern void diff_unmerge(struct diff_options *,
const char *path,
Expand Down
4 changes: 2 additions & 2 deletions revision.c
View file
Expand Up @@ -259,7 +259,7 @@ static int tree_difference = REV_TREE_SAME;
static void file_add_remove(struct diff_options *options,
int addremove, unsigned mode,
const unsigned char *sha1,
const char *base, const char *path)
const char *fullpath)
{
int diff = REV_TREE_DIFFERENT;

Expand All @@ -285,7 +285,7 @@ static void file_change(struct diff_options *options,
unsigned old_mode, unsigned new_mode,
const unsigned char *old_sha1,
const unsigned char *new_sha1,
const char *base, const char *path)
const char *fullpath)
{
tree_difference = REV_TREE_DIFFERENT;
DIFF_OPT_SET(options, HAS_CHANGES);
Expand Down
27 changes: 22 additions & 5 deletions tree-diff.c
View file
Expand Up @@ -15,6 +15,15 @@ static char *malloc_base(const char *base, int baselen, const char *path, int pa
return newbase;
}

static char *malloc_fullname(const char *base, int baselen, const char *path, int pathlen)
{
char *fullname = xmalloc(baselen + pathlen + 1);
memcpy(fullname, base, baselen);
memcpy(fullname + baselen, path, pathlen);
fullname[baselen + pathlen] = 0;
return fullname;
}

static void show_entry(struct diff_options *opt, const char *prefix, struct tree_desc *desc,
const char *base, int baselen);

Expand All @@ -24,6 +33,7 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const
const char *path1, *path2;
const unsigned char *sha1, *sha2;
int cmp, pathlen1, pathlen2;
char *fullname;

sha1 = tree_entry_extract(t1, &path1, &mode1);
sha2 = tree_entry_extract(t2, &path2, &mode2);
Expand Down Expand Up @@ -55,15 +65,20 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const
if (DIFF_OPT_TST(opt, RECURSIVE) && S_ISDIR(mode1)) {
int retval;
char *newbase = malloc_base(base, baselen, path1, pathlen1);
if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE))
if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE)) {
newbase[baselen + pathlen1] = 0;
opt->change(opt, mode1, mode2,
sha1, sha2, base, path1);
sha1, sha2, newbase);
newbase[baselen + pathlen1] = '/';
}
retval = diff_tree_sha1(sha1, sha2, newbase, opt);
free(newbase);
return retval;
}

opt->change(opt, mode1, mode2, sha1, sha2, base, path1);
fullname = malloc_fullname(base, baselen, path1, pathlen1);
opt->change(opt, mode1, mode2, sha1, sha2, fullname);
free(fullname);
return 0;
}

Expand Down Expand Up @@ -205,10 +220,10 @@ static void show_entry(struct diff_options *opt, const char *prefix, struct tree
unsigned mode;
const char *path;
const unsigned char *sha1 = tree_entry_extract(desc, &path, &mode);
int pathlen = tree_entry_len(path, sha1);

if (DIFF_OPT_TST(opt, RECURSIVE) && S_ISDIR(mode)) {
enum object_type type;
int pathlen = tree_entry_len(path, sha1);
char *newbase = malloc_base(base, baselen, path, pathlen);
struct tree_desc inner;
void *tree;
Expand All @@ -224,7 +239,9 @@ static void show_entry(struct diff_options *opt, const char *prefix, struct tree
free(tree);
free(newbase);
} else {
opt->add_remove(opt, prefix[0], mode, sha1, base, path);
char *fullname = malloc_fullname(base, baselen, path, pathlen);
opt->add_remove(opt, prefix[0], mode, sha1, fullname);
free(fullname);
}
}

Expand Down

0 comments on commit fd55a19

Please sign in to comment.