On Sun, Jun 10, 2018 at 3:56 AM, René Scharfe <l.s.r@xxxxxx> wrote:
> Am 10.11.2017 um 20:05 schrieb Elijah Newren:
>> +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.
> -- >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 <l.s.r@xxxxxx>
> ---
> 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 <newren@xxxxxxxxx>
