Re: UB in compath

I apologize for letting this slip by.

I've been using it in my personal build since you first posted it and haven't noticed any issues.

Even better, it does look like a correct fix.  And it's not worth trying to write a specific test case for it.  Ralph, do you agree?

David


Philipp wrote:

> 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;
> >                  }