[OE-core] [PATCH] inetutils: Fix abort on invalid files

Ricardo Ribalda Delgado ricardo at ribalda.com
Fri Jul 19 06:12:42 UTC 2019


Hi Khem

Indeed is due to that, but infortunately it is part of arpa/tftp.h .
So the propossed solutionI believe that it is simpler.

https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html

Also is what I have been suggested in #gcc


Thanks!

On Fri, Jul 19, 2019 at 12:39 AM Khem Raj <raj.khem at gmail.com> wrote:
>
> On Thu, Jul 18, 2019 at 2:10 PM Ricardo Ribalda Delgado
> <ricardo at ribalda.com> wrote:
> >
> > Hi Khem
> >
> > I think the issue is that __memcpy_chk wrongly assume that the target
> > size is 0. The origin size is calculated fine:
> >
> > https://godbolt.org/z/qTaDWP
> >
>             char tu_data[0];    /* data or error string */
>
> that seems an old stype variable length array
> can you change it to char tu_data[] and see if it helps
>
> > Thanks!
> >
> > On Thu, Jul 18, 2019 at 11:02 PM Khem Raj <raj.khem at gmail.com> wrote:
> > >
> > > On Thu, Jul 18, 2019 at 12:46 PM Ricardo Ribalda Delgado
> > > <ricardo at ribalda.com> wrote:
> > > >
> > > > When the code is compiled with  "-fstack-protector-strong
> > > > -D_FORTIFY_SOURCE=2", everytime ftpfd is asked for a non existent file,
> > > > it crashes with the following error:
> > > >
> > > > *** buffer overflow detected ***:
> > > > Aborted
> > > >
> > > > This seems to be a bug/feature of gcc. A bug has been open on their
> > > > bugzilla, and also inetutils have been posted with the proposed patch.
> > > >
> > > > Without this patch, pxelinux fails to boot because it keeps asking the
> > > > server for the pxelinux.cfg/00-01-02-03-04 and never jumps to /default.
> > > >
> > > > Signed-off-by: Ricardo Ribalda Delgado <ricardo at ribalda.com>
> > > > ---
> > > >  .../inetutils/fix-buffer-fortify-tfpt.patch   | 25 +++++++++++++++++++
> > > >  .../inetutils/inetutils_1.9.4.bb              |  1 +
> > > >  2 files changed, 26 insertions(+)
> > > >  create mode 100644 meta/recipes-connectivity/inetutils/inetutils/fix-buffer-fortify-tfpt.patch
> > > >
> > > > diff --git a/meta/recipes-connectivity/inetutils/inetutils/fix-buffer-fortify-tfpt.patch b/meta/recipes-connectivity/inetutils/inetutils/fix-buffer-fortify-tfpt.patch
> > > > new file mode 100644
> > > > index 0000000000..a91913cb51
> > > > --- /dev/null
> > > > +++ b/meta/recipes-connectivity/inetutils/inetutils/fix-buffer-fortify-tfpt.patch
> > > > @@ -0,0 +1,25 @@
> > > > +tftpd: Fix abort on error path
> > > > +
> > > > +When trying to fetch a non existent file, the app crashes with:
> > > > +
> > > > +*** buffer overflow detected ***:
> > > > +Aborted
> > > > +
> > > > +
> > > > +Upstream-Status: Submitted [https://www.mail-archive.com/bug-inetutils@gnu.org/msg03036.html https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91205]
> > > > +Signed-off-by: Ricardo Ribalda Delgado <ricardo at ribalda.com>
> > > > +diff --git a/src/tftpd.c b/src/tftpd.c
> > > > +index 56002a0..144012f 100644
> > > > +--- a/src/tftpd.c
> > > > ++++ b/src/tftpd.c
> > > > +@@ -864,9 +864,8 @@ nak (int error)
> > > > +       pe->e_msg = strerror (error - 100);
> > > > +       tp->th_code = EUNDEF;   /* set 'undef' errorcode */
> > > > +     }
> > > > +-  strcpy (tp->th_msg, pe->e_msg);
> > > > +   length = strlen (pe->e_msg);
> > >
> > > I wonder if length calculation is a problem as well here, if so then
> > > it would need
> > > correcting because it used in code below as well.
> > >
> > > > +-  tp->th_msg[length] = '\0';
> > > > ++  memcpy(tp->th_msg, pe->e_msg, length + 1);
> > > > +   length += 5;
> > > > +   if (sendto (peer, buf, length, 0, (struct sockaddr *) &from, fromlen) != length)
> > > > +     syslog (LOG_ERR, "nak: %m\n");
> > > > diff --git a/meta/recipes-connectivity/inetutils/inetutils_1.9.4.bb b/meta/recipes-connectivity/inetutils/inetutils_1.9.4.bb
> > > > index ac2e017d8b..684fbe09e1 100644
> > > > --- a/meta/recipes-connectivity/inetutils/inetutils_1.9.4.bb
> > > > +++ b/meta/recipes-connectivity/inetutils/inetutils_1.9.4.bb
> > > > @@ -22,6 +22,7 @@ SRC_URI = "${GNU_MIRROR}/inetutils/inetutils-${PV}.tar.gz \
> > > >             file://inetutils-1.9-PATH_PROCNET_DEV.patch \
> > > >             file://inetutils-only-check-pam_appl.h-when-pam-enabled.patch \
> > > >             file://0001-rcp-fix-to-work-with-large-files.patch \
> > > > +           file://fix-buffer-fortify-tfpt.patch \
> > > >  "
> > > >
> > > >  SRC_URI[md5sum] = "04852c26c47cc8c6b825f2b74f191f52"
> > > > --
> > > > 2.20.1
> > > >
> > > > --
> > > > _______________________________________________
> > > > Openembedded-core mailing list
> > > > Openembedded-core at lists.openembedded.org
> > > > http://lists.openembedded.org/mailman/listinfo/openembedded-core
> >
> >
> >
> > --
> > Ricardo Ribalda
> > Thanks!
> >
> > On Thu, Jul 18, 2019 at 11:02 PM Khem Raj <raj.khem at gmail.com> wrote:
> > >
> > > On Thu, Jul 18, 2019 at 12:46 PM Ricardo Ribalda Delgado
> > > <ricardo at ribalda.com> wrote:
> > > >
> > > > When the code is compiled with  "-fstack-protector-strong
> > > > -D_FORTIFY_SOURCE=2", everytime ftpfd is asked for a non existent file,
> > > > it crashes with the following error:
> > > >
> > > > *** buffer overflow detected ***:
> > > > Aborted
> > > >
> > > > This seems to be a bug/feature of gcc. A bug has been open on their
> > > > bugzilla, and also inetutils have been posted with the proposed patch.
> > > >
> > > > Without this patch, pxelinux fails to boot because it keeps asking the
> > > > server for the pxelinux.cfg/00-01-02-03-04 and never jumps to /default.
> > > >
> > > > Signed-off-by: Ricardo Ribalda Delgado <ricardo at ribalda.com>
> > > > ---
> > > >  .../inetutils/fix-buffer-fortify-tfpt.patch   | 25 +++++++++++++++++++
> > > >  .../inetutils/inetutils_1.9.4.bb              |  1 +
> > > >  2 files changed, 26 insertions(+)
> > > >  create mode 100644 meta/recipes-connectivity/inetutils/inetutils/fix-buffer-fortify-tfpt.patch
> > > >
> > > > diff --git a/meta/recipes-connectivity/inetutils/inetutils/fix-buffer-fortify-tfpt.patch b/meta/recipes-connectivity/inetutils/inetutils/fix-buffer-fortify-tfpt.patch
> > > > new file mode 100644
> > > > index 0000000000..a91913cb51
> > > > --- /dev/null
> > > > +++ b/meta/recipes-connectivity/inetutils/inetutils/fix-buffer-fortify-tfpt.patch
> > > > @@ -0,0 +1,25 @@
> > > > +tftpd: Fix abort on error path
> > > > +
> > > > +When trying to fetch a non existent file, the app crashes with:
> > > > +
> > > > +*** buffer overflow detected ***:
> > > > +Aborted
> > > > +
> > > > +
> > > > +Upstream-Status: Submitted [https://www.mail-archive.com/bug-inetutils@gnu.org/msg03036.html https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91205]
> > > > +Signed-off-by: Ricardo Ribalda Delgado <ricardo at ribalda.com>
> > > > +diff --git a/src/tftpd.c b/src/tftpd.c
> > > > +index 56002a0..144012f 100644
> > > > +--- a/src/tftpd.c
> > > > ++++ b/src/tftpd.c
> > > > +@@ -864,9 +864,8 @@ nak (int error)
> > > > +       pe->e_msg = strerror (error - 100);
> > > > +       tp->th_code = EUNDEF;   /* set 'undef' errorcode */
> > > > +     }
> > > > +-  strcpy (tp->th_msg, pe->e_msg);
> > > > +   length = strlen (pe->e_msg);
> > >
> > > I wonder if length calculation is a problem as well here, if so then
> > > it would need
> > > correcting because it used in code below as well.
> > >
> > > > +-  tp->th_msg[length] = '\0';
> > > > ++  memcpy(tp->th_msg, pe->e_msg, length + 1);
> > > > +   length += 5;
> > > > +   if (sendto (peer, buf, length, 0, (struct sockaddr *) &from, fromlen) != length)
> > > > +     syslog (LOG_ERR, "nak: %m\n");
> > > > diff --git a/meta/recipes-connectivity/inetutils/inetutils_1.9.4.bb b/meta/recipes-connectivity/inetutils/inetutils_1.9.4.bb
> > > > index ac2e017d8b..684fbe09e1 100644
> > > > --- a/meta/recipes-connectivity/inetutils/inetutils_1.9.4.bb
> > > > +++ b/meta/recipes-connectivity/inetutils/inetutils_1.9.4.bb
> > > > @@ -22,6 +22,7 @@ SRC_URI = "${GNU_MIRROR}/inetutils/inetutils-${PV}.tar.gz \
> > > >             file://inetutils-1.9-PATH_PROCNET_DEV.patch \
> > > >             file://inetutils-only-check-pam_appl.h-when-pam-enabled.patch \
> > > >             file://0001-rcp-fix-to-work-with-large-files.patch \
> > > > +           file://fix-buffer-fortify-tfpt.patch \
> > > >  "
> > > >
> > > >  SRC_URI[md5sum] = "04852c26c47cc8c6b825f2b74f191f52"
> > > > --
> > > > 2.20.1
> > > >
> > > > --
> > > > _______________________________________________
> > > > Openembedded-core mailing list
> > > > Openembedded-core at lists.openembedded.org
> > > > http://lists.openembedded.org/mailman/listinfo/openembedded-core
> >
> >
> >
> > --
> > Ricardo Ribalda
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core at lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core



-- 
Ricardo Ribalda


More information about the Openembedded-core mailing list