Fix buffer overflow in git diff

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>

diff --git a/diff-lib.c b/diff-lib.c
index b17722d66a7008b822125bb96ab1b7921e8b627b..e7eaff9a68ccbcc692522c9956f0dae9af45f3f1 100644 (file)
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -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,
                        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);
                        continue;
                }
                changed = ce_match_stat(ce, &st, ce_option);


@@ -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),
                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);
 
        }
        diffcore_std(&revs->diffopt);


@@ -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,
                                 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,
 }
 
 static int get_stat_data(struct cache_entry *ce,


@@ -312,7 +312,7 @@ static int show_modified(struct oneway_unpack_data *cbdata,
                return 0;
 
        diff_change(&revs->diffopt, oldmode, mode,
                return 0;
 
        diff_change(&revs->diffopt, oldmode, mode,

-                   old->sha1, sha1, old->name, NULL);
+                   old->sha1, sha1, old->name);

        return 0;
 }
 
        return 0;
 }
 

diff --git a/diff.c b/diff.c
index 78c4d3a35a975d0ffef51dbe999d14938d0a90e3..386de826d3a050e5dfce55636fc2e65129b6da2b 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -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,
 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))
        struct diff_filespec *one, *two;
 
        if (DIFF_OPT_TST(options, IGNORE_SUBMODULES) && S_ISGITLINK(mode))


@@ -3380,9 +3379,6 @@ void diff_addremove(struct diff_options *options,
                addremove = (addremove == '+' ? '-' :
                             addremove == '-' ? '+' : addremove);
 
                addremove = (addremove == '+' ? '-' :
                             addremove == '-' ? '+' : addremove);
 

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

        if (options->prefix &&
            strncmp(concatpath, options->prefix, options->prefix_length))
                return;
        if (options->prefix &&
            strncmp(concatpath, options->prefix, options->prefix_length))
                return;


@@ -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,
                 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)
        struct diff_filespec *one, *two;
 
        if (DIFF_OPT_TST(options, IGNORE_SUBMODULES) && S_ISGITLINK(old_mode)


@@ -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;
        }
                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))
 
        if (options->prefix &&
            strncmp(concatpath, options->prefix, options->prefix_length))

diff --git a/diff.h b/diff.h
index 5dc0cb595b64868ca8b8e26e5311d2df102c89e0..50fb5ddb0bec02b0cd5498d6ecc37d44bf874476 100644 (file)
--- a/diff.h
+++ b/diff.h
@@ -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,
                 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,
 
 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);
 
 typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
                struct diff_options *options, void *data);


@@ -164,14 +164,13 @@ extern void diff_addremove(struct diff_options *,
                           int addremove,
                           unsigned mode,
                           const unsigned char *sha1,
                           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,
 
 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,
 
 extern void diff_unmerge(struct diff_options *,
                         const char *path,

diff --git a/revision.c b/revision.c
index fc667552592daa3c894d0df4e97469d1809dbf27..8dc3ca7bf696c14ad73f4ebe0fe845cba298c20e 100644 (file)
--- a/revision.c
+++ b/revision.c
@@ -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,
 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;
 
 {
        int diff = REV_TREE_DIFFERENT;
 


@@ -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,
                 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);
 {
        tree_difference = REV_TREE_DIFFERENT;
        DIFF_OPT_SET(options, HAS_CHANGES);

diff --git a/tree-diff.c b/tree-diff.c
index e1e2e6c6ce3c4effffb26e9aed7c6bdd2b00a5ae..bbb126fc46cfb28a0bc92cc0842c0dc72017751d 100644 (file)
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -15,6 +15,15 @@ static char *malloc_base(const char *base, int baselen, const char *path, int pa
        return newbase;
 }
 
        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);
 
 static void show_entry(struct diff_options *opt, const char *prefix, struct tree_desc *desc,
                       const char *base, int baselen);
 


@@ -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;
        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);
 
        sha1 = tree_entry_extract(t1, &path1, &mode1);
        sha2 = tree_entry_extract(t2, &path2, &mode2);


@@ -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, 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,
                        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;
        }
 
                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;
 }
 
        return 0;
 }
 


@@ -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);
        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;
 
        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;
                char *newbase = malloc_base(base, baselen, path, pathlen);
                struct tree_desc inner;
                void *tree;


@@ -224,7 +239,9 @@ static void show_entry(struct diff_options *opt, const char *prefix, struct tree
                free(tree);
                free(newbase);
        } else {
                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);

        }
 }
 
        }
 }