Re: UB in compath

Hi

Has anyone time to look at this and maybe commit it?

Philipp

[2026-05-24 13:21] Philipp <[email protected]>
> Hi
>
> I had tried to move some mails to a different folder and noticed that
> refile changed the target directory. The path '@../bla/blub' was changed
> to something like '../blaxzub'  So I checked the code and found strcpy
> in compath (sbr/path.c). This function change a given path ìnplace.
> But using strcpy on overlapping objects is UB.
>
> I tried to write a test, but couldn't reproduce the behaivior with the
> test suite. I'm not sure if this is because the bugs only triggert
> with my full mailbox or because I use a different version of path.c.
>
> But the code still contains UB which sould be fixed. I have written
> a quick and dirty fix with strlen and memmove. It would be better
> to avoid recalculation of the strlen or avoid the inplace manipulation
> of the string.
>
> Philipp
>
> diff /home/satanist/src/nmh
> path + /home/satanist/src/nmh
> commit - 9ce1475266997843ad456da6d2940340f31d9687
> blob - 08de38989c6501c201b28a382b5a37ed143d79ad
> file + sbr/path.c
> --- sbr/path.c
> +++ sbr/path.c
> @@ -143,6 +143,7 @@ static void
>  compath (char *f)
>  {
>      char *cp, *dp;
> +    size_t len;
>  
>      if (*f != '/')
>  	return;
> @@ -162,7 +163,8 @@ compath (char *f)
>              case '/':
>                  for (dp = cp; *dp == '/'; dp++)
>                      continue;
> -                strcpy (cp--, dp);
> +                len = strlen(dp) + 1;
> +                memmove (cp--, dp, len);
>                  continue;
>  
>              case '.':
> @@ -187,12 +189,14 @@ compath (char *f)
>                              break;
>                      if (dp <= f)
>                          dp = f;
> -                    strcpy (dp, cp + LEN(PWD) - 1);
> +                    len = strlen(cp + LEN(PWD) - 1) + 1;
> +                    memmove (dp, cp + LEN(PWD) - 1, len);
>                      cp = dp;
>                      continue;
>                  }
>                  if (has_prefix(cp, CWD)) {
> -                    strcpy (cp - 1, cp + LEN(CWD) - 1);
> +                    len = strlen(cp + LEN(CWD) - 1) + 1;
> +                    memmove (cp - 1, cp + LEN(CWD) - 1, len);
>                      cp--;
>                      continue;
>                  }
>
>