[PATCH] ed: don't use memcpy with overlapping memory regions

Denys Vlasenko vda.linux at googlemail.com
Thu Jan 5 15:24:46 UTC 2023


Applied, thank you

On Thu, Dec 22, 2022 at 3:30 PM Sören Tempel <soeren at soeren-tempel.net> wrote:
>
> PING.
>
> Any love for good old ed(1)?
>
> Sören Tempel <soeren at soeren-tempel.net> wrote:
> > Pinging again as this is still unfixed and the proposed fix is rather trivial.
> >
> > Sören Tempel <soeren at soeren-tempel.net> wrote:
> > > Ping.
> > >
> > > soeren at soeren-tempel.net wrote:
> > > > From: Sören Tempel <soeren+git at soeren-tempel.net>
> > > >
> > > > The memcpy invocations in the subCommand function, modified by this
> > > > commit, previously used memcpy with overlapping memory regions. This is
> > > > undefined behavior. On Alpine Linux, it causes BusyBox ed to crash since
> > > > we compile BusyBox with -D_FORTIFY_SOURCE=2 and our fortify-headers
> > > > implementation catches this source of undefined behavior [0]. The issue
> > > > can only be triggered if the replacement string is the same size or
> > > > shorter than the old string.
> > > >
> > > > Looking at the code, it seems to me that a memmove(3) is what was
> > > > actually intended here, this commit modifies the code accordingly.
> > > >
> > > > [0]: https://gitlab.alpinelinux.org/alpine/aports/-/issues/13504
> > > > ---
> > > >  editors/ed.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/editors/ed.c b/editors/ed.c
> > > > index 209ce9942..4a84f7433 100644
> > > > --- a/editors/ed.c
> > > > +++ b/editors/ed.c
> > > > @@ -720,7 +720,7 @@ static void subCommand(const char *cmd, int num1, int num2)
> > > >           if (deltaLen <= 0) {
> > > >                   memcpy(&lp->data[offset], newStr, newLen);
> > > >                   if (deltaLen) {
> > > > -                         memcpy(&lp->data[offset + newLen],
> > > > +                         memmove(&lp->data[offset + newLen],
> > > >                                   &lp->data[offset + oldLen],
> > > >                                   lp->len - offset - oldLen);
> > > >
> > > > _______________________________________________
> > > > busybox mailing list
> > > > busybox at busybox.net
> > > > http://lists.busybox.net/mailman/listinfo/busybox
> > > _______________________________________________
> > > busybox mailing list
> > > busybox at busybox.net
> > > http://lists.busybox.net/mailman/listinfo/busybox
> > _______________________________________________
> > busybox mailing list
> > busybox at busybox.net
> > http://lists.busybox.net/mailman/listinfo/busybox
> _______________________________________________
> busybox mailing list
> busybox at busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox


More information about the busybox mailing list