Re: [PATCH 24/30] merge-recursive: Add computation of collisions due to dir rename & merging
From: Elijah Newren <hidden>
Date: 2018-06-11 15:03:04
On Sun, Jun 10, 2018 at 3:56 AM, René Scharfe [off-list ref] wrote:
Am 10.11.2017 um 20:05 schrieb Elijah Newren:quoted
+static struct dir_rename_entry *check_dir_renamed(const char *path, + struct hashmap *dir_renames) { + char temp[PATH_MAX]; + char *end; + struct dir_rename_entry *entry; + + strcpy(temp, path); + while ((end = strrchr(temp, '/'))) { + *end = '\0'; + entry = dir_rename_find_entry(dir_renames, temp); + if (entry) + return entry; + } + return NULL; +}The value of PATH_MAX is platform-dependent, so it's easy to exceed when doing cross-platform development. It's also not a hard limit on most operating systems, not even on Windows. Further reading: https://insanecoding.blogspot.com/2007/11/pathmax-simply-isnt.html So using a fixed buffer is not a good idea, and writing to it without checking is dangerous. Here's a fix:
Thanks for the pointers, and for providing a fix.
quoted hunk ↗ jump to hunk
-- >8 -- Subject: [PATCH] merge-recursive: use xstrdup() instead of fixed buffer Paths can be longer than PATH_MAX. Avoid a buffer overrun in check_dir_renamed() by using xstrdup() to make a private copy safely. Signed-off-by: Rene Scharfe <redacted> --- merge-recursive.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)diff --git a/merge-recursive.c b/merge-recursive.c index ac27abbd4c..db708176c5 100644 --- a/merge-recursive.c +++ b/merge-recursive.c@@ -2211,18 +2211,18 @@ static struct hashmap *get_directory_renames(struct diff_queue_struct *pairs, static struct dir_rename_entry *check_dir_renamed(const char *path, struct hashmap *dir_renames) { - char temp[PATH_MAX]; + char *temp = xstrdup(path); char *end; - struct dir_rename_entry *entry; + struct dir_rename_entry *entry = NULL;; - strcpy(temp, path); while ((end = strrchr(temp, '/'))) { *end = '\0'; entry = dir_rename_find_entry(dir_renames, temp); if (entry) - return entry; + break; } - return NULL; + free(temp); + return entry; } static void compute_collisions(struct hashmap *collisions, --2.17.1
Reviewed-by: Elijah Newren <redacted>