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>maint
							parent
							
								
									620e2bb937
								
							
						
					
					
						commit
						fd55a19eb1
					
				|  | @ -171,7 +171,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option) | ||||||
| 			if (silent_on_removed) | 			if (silent_on_removed) | ||||||
| 				continue; | 				continue; | ||||||
| 			diff_addremove(&revs->diffopt, '-', ce->ce_mode, | 			diff_addremove(&revs->diffopt, '-', ce->ce_mode, | ||||||
| 				       ce->sha1, ce->name, NULL); | 				       ce->sha1, ce->name); | ||||||
| 			continue; | 			continue; | ||||||
| 		} | 		} | ||||||
| 		changed = ce_match_stat(ce, &st, ce_option); | 		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); | 		newmode = ce_mode_from_stat(ce, st.st_mode); | ||||||
| 		diff_change(&revs->diffopt, oldmode, newmode, | 		diff_change(&revs->diffopt, oldmode, newmode, | ||||||
| 			    ce->sha1, (changed ? null_sha1 : ce->sha1), | 			    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) | 				 const unsigned char *sha1, unsigned int mode) | ||||||
| { | { | ||||||
| 	diff_addremove(&revs->diffopt, prefix[0], 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; | 		return 0; | ||||||
|  |  | ||||||
| 	diff_change(&revs->diffopt, oldmode, mode, | 	diff_change(&revs->diffopt, oldmode, mode, | ||||||
| 		    old->sha1, sha1, old->name, NULL); | 		    old->sha1, sha1, old->name); | ||||||
| 	return 0; | 	return 0; | ||||||
| } | } | ||||||
|  |  | ||||||
|  |  | ||||||
							
								
								
									
										11
									
								
								diff.c
								
								
								
								
							
							
						
						
									
										11
									
								
								diff.c
								
								
								
								
							|  | @ -3356,9 +3356,8 @@ int diff_result_code(struct diff_options *opt, int status) | ||||||
| void diff_addremove(struct diff_options *options, | void diff_addremove(struct diff_options *options, | ||||||
| 		    int addremove, unsigned mode, | 		    int addremove, unsigned mode, | ||||||
| 		    const unsigned char *sha1, | 		    const unsigned char *sha1, | ||||||
| 		    const char *base, const char *path) | 		    const char *concatpath) | ||||||
| { | { | ||||||
| 	char concatpath[PATH_MAX]; |  | ||||||
| 	struct diff_filespec *one, *two; | 	struct diff_filespec *one, *two; | ||||||
|  |  | ||||||
| 	if (DIFF_OPT_TST(options, IGNORE_SUBMODULES) && S_ISGITLINK(mode)) | 	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 && | 	if (options->prefix && | ||||||
| 	    strncmp(concatpath, options->prefix, options->prefix_length)) | 	    strncmp(concatpath, options->prefix, options->prefix_length)) | ||||||
| 		return; | 		return; | ||||||
|  | @ -3403,9 +3399,8 @@ void diff_change(struct diff_options *options, | ||||||
| 		 unsigned old_mode, unsigned new_mode, | 		 unsigned old_mode, unsigned new_mode, | ||||||
| 		 const unsigned char *old_sha1, | 		 const unsigned char *old_sha1, | ||||||
| 		 const unsigned char *new_sha1, | 		 const unsigned char *new_sha1, | ||||||
| 		 const char *base, const char *path) | 		 const char *concatpath) | ||||||
| { | { | ||||||
| 	char concatpath[PATH_MAX]; |  | ||||||
| 	struct diff_filespec *one, *two; | 	struct diff_filespec *one, *two; | ||||||
|  |  | ||||||
| 	if (DIFF_OPT_TST(options, IGNORE_SUBMODULES) && S_ISGITLINK(old_mode) | 	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 = old_mode; old_mode = new_mode; new_mode = tmp; | ||||||
| 		tmp_c = old_sha1; old_sha1 = new_sha1; new_sha1 = tmp_c; | 		tmp_c = old_sha1; old_sha1 = new_sha1; new_sha1 = tmp_c; | ||||||
| 	} | 	} | ||||||
| 	if (!path) path = ""; |  | ||||||
| 	sprintf(concatpath, "%s%s", base, path); |  | ||||||
|  |  | ||||||
| 	if (options->prefix && | 	if (options->prefix && | ||||||
| 	    strncmp(concatpath, options->prefix, options->prefix_length)) | 	    strncmp(concatpath, options->prefix, options->prefix_length)) | ||||||
|  |  | ||||||
							
								
								
									
										9
									
								
								diff.h
								
								
								
								
							
							
						
						
									
										9
									
								
								diff.h
								
								
								
								
							|  | @ -14,12 +14,12 @@ typedef void (*change_fn_t)(struct diff_options *options, | ||||||
| 		 unsigned old_mode, unsigned new_mode, | 		 unsigned old_mode, unsigned new_mode, | ||||||
| 		 const unsigned char *old_sha1, | 		 const unsigned char *old_sha1, | ||||||
| 		 const unsigned char *new_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, | typedef void (*add_remove_fn_t)(struct diff_options *options, | ||||||
| 		    int addremove, unsigned mode, | 		    int addremove, unsigned mode, | ||||||
| 		    const unsigned char *sha1, | 		    const unsigned char *sha1, | ||||||
| 		    const char *base, const char *path); | 		    const char *fullpath); | ||||||
|  |  | ||||||
| typedef void (*diff_format_fn_t)(struct diff_queue_struct *q, | typedef void (*diff_format_fn_t)(struct diff_queue_struct *q, | ||||||
| 		struct diff_options *options, void *data); | 		struct diff_options *options, void *data); | ||||||
|  | @ -164,14 +164,13 @@ extern void diff_addremove(struct diff_options *, | ||||||
| 			   int addremove, | 			   int addremove, | ||||||
| 			   unsigned mode, | 			   unsigned mode, | ||||||
| 			   const unsigned char *sha1, | 			   const unsigned char *sha1, | ||||||
| 			   const char *base, | 			   const char *fullpath); | ||||||
| 			   const char *path); |  | ||||||
|  |  | ||||||
| extern void diff_change(struct diff_options *, | extern void diff_change(struct diff_options *, | ||||||
| 			unsigned mode1, unsigned mode2, | 			unsigned mode1, unsigned mode2, | ||||||
| 			const unsigned char *sha1, | 			const unsigned char *sha1, | ||||||
| 			const unsigned char *sha2, | 			const unsigned char *sha2, | ||||||
| 			const char *base, const char *path); | 			const char *fullpath); | ||||||
|  |  | ||||||
| extern void diff_unmerge(struct diff_options *, | extern void diff_unmerge(struct diff_options *, | ||||||
| 			 const char *path, | 			 const char *path, | ||||||
|  |  | ||||||
|  | @ -259,7 +259,7 @@ static int tree_difference = REV_TREE_SAME; | ||||||
| static void file_add_remove(struct diff_options *options, | static void file_add_remove(struct diff_options *options, | ||||||
| 		    int addremove, unsigned mode, | 		    int addremove, unsigned mode, | ||||||
| 		    const unsigned char *sha1, | 		    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, | 		 unsigned old_mode, unsigned new_mode, | ||||||
| 		 const unsigned char *old_sha1, | 		 const unsigned char *old_sha1, | ||||||
| 		 const unsigned char *new_sha1, | 		 const unsigned char *new_sha1, | ||||||
| 		 const char *base, const char *path) | 		 const char *fullpath) | ||||||
| { | { | ||||||
| 	tree_difference = REV_TREE_DIFFERENT; | 	tree_difference = REV_TREE_DIFFERENT; | ||||||
| 	DIFF_OPT_SET(options, HAS_CHANGES); | 	DIFF_OPT_SET(options, HAS_CHANGES); | ||||||
|  |  | ||||||
							
								
								
									
										27
									
								
								tree-diff.c
								
								
								
								
							
							
						
						
									
										27
									
								
								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, | static void show_entry(struct diff_options *opt, const char *prefix, struct tree_desc *desc, | ||||||
| 		       const char *base, int baselen); | 		       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 char *path1, *path2; | ||||||
| 	const unsigned char *sha1, *sha2; | 	const unsigned char *sha1, *sha2; | ||||||
| 	int cmp, pathlen1, pathlen2; | 	int cmp, pathlen1, pathlen2; | ||||||
|  | 	char *fullname; | ||||||
|  |  | ||||||
| 	sha1 = tree_entry_extract(t1, &path1, &mode1); | 	sha1 = tree_entry_extract(t1, &path1, &mode1); | ||||||
| 	sha2 = tree_entry_extract(t2, &path2, &mode2); | 	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)) { | 	if (DIFF_OPT_TST(opt, RECURSIVE) && S_ISDIR(mode1)) { | ||||||
| 		int retval; | 		int retval; | ||||||
| 		char *newbase = malloc_base(base, baselen, path1, pathlen1); | 		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); | 		retval = diff_tree_sha1(sha1, sha2, newbase, opt); | ||||||
| 		free(newbase); | 		free(newbase); | ||||||
| 		return retval; | 		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; | 	unsigned mode; | ||||||
| 	const char *path; | 	const char *path; | ||||||
| 	const unsigned char *sha1 = tree_entry_extract(desc, &path, &mode); | 	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)) { | 	if (DIFF_OPT_TST(opt, RECURSIVE) && S_ISDIR(mode)) { | ||||||
| 		enum object_type type; | 		enum object_type type; | ||||||
| 		int pathlen = tree_entry_len(path, sha1); |  | ||||||
| 		char *newbase = malloc_base(base, baselen, path, pathlen); | 		char *newbase = malloc_base(base, baselen, path, pathlen); | ||||||
| 		struct tree_desc inner; | 		struct tree_desc inner; | ||||||
| 		void *tree; | 		void *tree; | ||||||
|  | @ -224,7 +239,9 @@ static void show_entry(struct diff_options *opt, const char *prefix, struct tree | ||||||
| 		free(tree); | 		free(tree); | ||||||
| 		free(newbase); | 		free(newbase); | ||||||
| 	} else { | 	} 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); | ||||||
| 	} | 	} | ||||||
| } | } | ||||||
|  |  | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue
	
	 Dmitry Potapov
						Dmitry Potapov