On Mon, 2020-07-20 at 20:33 -0700, Khem Raj wrote: > > > On Mon, Jul 20, 2020 at 6:14 PM Julius Hemanth Pitti (jpitti) < > [email protected]> wrote: > > On Mon, 2020-07-20 at 15:48 -0700, Khem Raj wrote: > > > On Mon, Jul 20, 2020 at 3:27 PM Julius Hemanth Pitti < > > > [email protected]> wrote: > > > > > > > > netoprintf() was not handling a case where > > > > return value of vsnprintf is greater than > > > > "size"(2nd argument), results in buffer overflow > > > > while adjusting "nfrontp" pointer to point > > > > beyond "netobuf" buffer. > > > > > > > > Here is one such case where "nfrontp" > > > > crossed boundaries of "netobuf", and > > > > pointing to another global variable. > > > > > > > > (gdb) p &netobuf[8255] > > > > $5 = 0x55c93afe8b1f <netobuf+8255> "" > > > > (gdb) p nfrontp > > > > $6 = 0x55c93afe8c20 <terminaltype> "\377" > > > > (gdb) p &terminaltype > > > > $7 = (char **) 0x55c93afe8c20 <terminaltype> > > > > (gdb) > > > > > > > > This resulted in crash of telnetd service > > > > with segmentation fault. > > > > > > > > > > it seems like one. Can you also reproduce it with something like > > > fedora ? > > > > > > > I looked at latest centos and ubuntu, their refactored code do not > > have > > this bug. > > I would prefer to share the refactored code which avoids this > problem
Centos/Fedora and ubuntu have their own refactored code. Their patches do not have much in common. If we decide to share their refactored code to avoid this bug, then I recommend Centos way as it's closer to our source. centos/fedora refactored their code a long time ago in patch "telnet- 0.17-sa-01-49.patch" (Ref: https://source.ipfire.org/source-3.x/4b3195d73b47d94d75630046871fcadd7bce89a9/telnet/patches/telnet-0.17-sa-01-49.patch ) I looked at their patch, and the way they solve it logically derives to the same results. There is one particular hunk in this patch, which I do not understand why is it done. Here is the snippet, diff -up netkit-telnet-0.17/telnetd/termstat.c.sa-01-49 netkit-telnet- 0.17/telnetd/termstat.c --- netkit-telnet-0.17/telnetd/termstat.c.sa-01-49 1999-12-12 15:59:45.000000000 +0100 +++ netkit-telnet-0.17/telnetd/termstat.c 2011-01-20 22:39:54.000000000 +0100 @@ -128,7 +128,6 @@ static int _terminit = 0; void localstat() { - void netflush(); int need_will_echo = 0; /* Unfortunately this patch does not have any commit message or comment which explains why we need this. Maybe it has something to do with previous patches in the series, perhaps some bug fix, which they squashed together in this patch? not sure though. If we decide to take above hunk as-is, how do we ensure this doesn't have unwanted effects? With this details, IMO, I think we shouldn't share just the part that avoid this problem, We should either drop all our current patches(in our recipe) and just pick all the patches from centos/Fedora or ubuntu, or we make a point fix of this issue like the patch I submitted. Please suggest. > > > > > > Signed-off-by: Julius Hemanth Pitti <[email protected]> > > > > --- > > > > ....c-Fix-buffer-overflow-in-netoprintf.patch | 56 > > > > +++++++++++++++++++ > > > > .../netkit-telnet/netkit-telnet_0.17.bb | 1 + > > > > 2 files changed, 57 insertions(+) > > > > create mode 100644 meta-networking/recipes-netkit/netkit- > > > > telnet/files/0001-telnetd-utility.c-Fix-buffer-overflow-in- > > > > netoprintf.patch > > > > > > > > diff --git a/meta-networking/recipes-netkit/netkit- > > > > telnet/files/0001-telnetd-utility.c-Fix-buffer-overflow-in- > > > > netoprintf.patch b/meta-networking/recipes-netkit/netkit- > > > > telnet/files/0001-telnetd-utility.c-Fix-buffer-overflow-in- > > > > netoprintf.patch > > > > new file mode 100644 > > > > index 000000000..8f983e40a > > > > --- /dev/null > > > > +++ b/meta-networking/recipes-netkit/netkit-telnet/files/0001- > > > > telnetd-utility.c-Fix-buffer-overflow-in-netoprintf.patch > > > > @@ -0,0 +1,56 @@ > > > > +From 9c81c8e5bc7782e8ae12c078615abc3c896059f2 Mon Sep 17 > > 00:00:00 > > > > 2001 > > > > +From: Julius Hemanth Pitti <[email protected]> > > > > +Date: Tue, 14 Jul 2020 22:34:19 -0700 > > > > +Subject: [PATCH] telnetd/utility.c: Fix buffer overflow in > > > > netoprintf > > > > + > > > > +As per man page of vsnprintf, when formated > > > > +string size is greater than "size"(2nd argument), > > > > +then vsnprintf returns size of formated string, > > > > +not "size"(2nd argument). > > > > + > > > > +netoprintf() was not handling a case where > > > > +return value of vsnprintf is greater than > > > > +"size"(2nd argument), results in buffer overflow > > > > +while adjusting "nfrontp" pointer to point > > > > +beyond "netobuf" buffer. > > > > + > > > > +Here is one such case where "nfrontp" > > > > +crossed boundaries of "netobuf", and > > > > +pointing to another global variable. > > > > + > > > > +(gdb) p &netobuf[8255] > > > > +$5 = 0x55c93afe8b1f <netobuf+8255> "" > > > > +(gdb) p nfrontp > > > > +$6 = 0x55c93afe8c20 <terminaltype> "\377" > > > > +(gdb) p &terminaltype > > > > +$7 = (char **) 0x55c93afe8c20 <terminaltype> > > > > +(gdb) > > > > + > > > > +This resulted in crash of telnetd service > > > > +with segmentation fault. > > > > + > > > > +Though this is DoS security bug, I couldn't > > > > +find any CVE ID for this. > > > > + > > > > +Upstream-Status: Pending > > > > + > > > > +Signed-off-by: Julius Hemanth Pitti <[email protected]> > > > > +--- > > > > + telnetd/utility.c | 2 +- > > > > + 1 file changed, 1 insertion(+), 1 deletion(-) > > > > + > > > > +diff --git a/telnetd/utility.c b/telnetd/utility.c > > > > +index b9a46a6..4811f14 100644 > > > > +--- a/telnetd/utility.c > > > > ++++ b/telnetd/utility.c > > > > +@@ -66,7 +66,7 @@ netoprintf(const char *fmt, ...) > > > > + len = vsnprintf(nfrontp, maxsize, fmt, ap); > > > > + va_end(ap); > > > > + > > > > +- if (len<0 || len==maxsize) { > > > > ++ if (len<0 || len>=maxsize) { > > > > + /* didn't fit */ > > > > + netflush(); > > > > + } > > > > +-- > > > > +2.19.1 > > > > diff --git a/meta-networking/recipes-netkit/netkit- > > telnet/netkit- > > > > telnet_0.17.bb b/meta-networking/recipes-netkit/netkit- > > > > telnet/netkit-telnet_0.17.bb > > > > index 0e92add63..08dd532b6 100644 > > > > --- a/meta-networking/recipes-netkit/netkit-telnet/netkit- > > > > telnet_0.17.bb > > > > +++ b/meta-networking/recipes-netkit/netkit-telnet/netkit- > > > > telnet_0.17.bb > > > > @@ -13,6 +13,7 @@ SRC_URI = " > > > > > > http://ftp.linux.org.uk/pub/linux/Networking/netkit/${BP}.tar.gz \ > > > > file://0001-telnet-telnetd-Fix-print-format- > > > > strings.patch \ > > > > file://0001-telnet-telnetd-Fix-deadlock-on- > > > > cleanup.patch \ > > > > file://CVE-2020-10188.patch \ > > > > + file://0001-telnetd-utility.c-Fix-buffer-overflow- > > in- > > > > netoprintf.patch \ > > > > " > > > > > > > > UPSTREAM_CHECK_URI = "${DEBIAN_MIRROR}/main/n/netkit-telnet/" > > > > -- > > > > 2.19.1 > > > >
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#85848): https://lists.openembedded.org/g/openembedded-devel/message/85848 Mute This Topic: https://lists.openembedded.org/mt/75693009/21656 Group Owner: [email protected] Unsubscribe: https://lists.openembedded.org/g/openembedded-devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
