From owner-freebsd-audit Sun Jul 22 21:56:34 2001 Delivered-To: freebsd-audit@freebsd.org Received: from obsecurity.dyndns.org (adsl-63-207-60-215.dsl.lsan03.pacbell.net [63.207.60.215]) by hub.freebsd.org (Postfix) with ESMTP id 569D737B401; Sun, 22 Jul 2001 21:56:22 -0700 (PDT) (envelope-from kris@obsecurity.org) Received: by obsecurity.dyndns.org (Postfix, from userid 1000) id E4D8B66CC1; Sun, 22 Jul 2001 21:56:20 -0700 (PDT) Date: Sun, 22 Jul 2001 21:56:20 -0700 From: Kris Kennaway To: Assar Westerlund Cc: Kris Kennaway , Matt Dillon , Ruslan Ermilov , audit@FreeBSD.org Subject: Re: [PATCH] Re: FreeBSD remote root exploit ? Message-ID: <20010722215619.A94874@xor.obsecurity.org> References: <20010719205948.D67829@sunbay.com> <200107191817.f6JIHSJ76262@earth.backplane.com> <20010719215957.A74024@sunbay.com> <200107191917.f6JJHwV77405@earth.backplane.com> <20010720100029.A30828@sunbay.com> <200107200932.f6K9WgZ88552@earth.backplane.com> <20010720143742.E65677@sunbay.com> <200107201717.f6KHHGa91142@earth.backplane.com> <20010722194031.A92249@jail-3.5> <5l66ck9wm7.fsf@assaris.sics.se> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-md5; protocol="application/pgp-signature"; boundary="KsGdsel6WgEHnImy" Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: <5l66ck9wm7.fsf@assaris.sics.se>; from assar@FreeBSD.ORG on Mon, Jul 23, 2001 at 05:01:52AM +0200 Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG --KsGdsel6WgEHnImy Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jul 23, 2001 at 05:01:52AM +0200, Assar Westerlund wrote: > Kris Kennaway writes: > > Okay, I've been reviewing the patch, and this immediately stood out to > > me: > >=20 > > int > > output_data(const char *format, ...) > > { > > va_list args; > > size_t remaining, ret; > >=20 > > va_start(args, format); > > remaining =3D BUFSIZ - (nfrontp - netobuf); > > /* try a netflush() if the room is too low */ > > if (strlen(format) > remaining || BUFSIZ / 4 > remaining) { > > ^^^^^^^^^^^^^^ > > =20 > > format is a format string which gets expanded by snprintf..how can > > that check be right to determine if the buffer is going to become > > full? >=20 > I think the point of that check is to flush the buffer if it's likely > that this output_data would otherwise fill it up. Note that the > vsnprintf() still only uses the remaining space in the buffer. Please review the following patch. I made the following changes: * output_data() and output_datalen() now guarantee that they output their entire inputs, and can handle input string which have strlen() > BUFSIZ * I removed the test against BUFSIZ / 4 -- as far as I could tell, this is only intended to pre-emptively flush the buffer if we are close to full, but not completely full yet. I'm not sure why this was needed. * Some places which called netflush() conditionally (e.g. if there is insufficient free space for the operation they're about to do) now use while() to guarantee this; previously they could overflow if the netflush() failed. Some of the netflush() calls seemed to only be 'advisory' and nothing depends on the flush actually taking place immediately. I've left these alone for now; perhaps they should be changed to flush the entire buffer. * I've commented out the -D report code in netflush() which recurses; we'll have to revisit that. I wanted to release the telnetd advisory tomorrow, so your swift review is appreciated. Thanks. Kris Index: telnetd/ext.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /mnt/ncvs/src/crypto/telnet/telnetd/ext.h,v retrieving revision 1.5 diff -u -r1.5 ext.h --- telnetd/ext.h 2001/07/19 17:48:57 1.5 +++ telnetd/ext.h 2001/07/23 03:37:03 @@ -120,7 +120,6 @@ localstat P((void)), flowstat P((void)), netclear P((void)), - netflush P((void)), #ifdef DIAGNOSTICS printoption P((char *, int)), printdata P((char *, char *, int)), @@ -159,6 +158,7 @@ getpty P((int *)), #endif login_tty P((int)), + netflush P((void)), spcset P((int, cc_t *, cc_t **)), stilloob P((int)), terminit P((void)), Index: telnetd/slc.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /mnt/ncvs/src/crypto/telnet/telnetd/slc.c,v retrieving revision 1.5 diff -u -r1.5 slc.c --- telnetd/slc.c 2000/07/16 05:52:45 1.5 +++ telnetd/slc.c 2001/07/23 03:37:20 @@ -176,7 +176,6 @@ register unsigned char **bufp; { register int len; - void netflush(); =20 /* * If a change has occured, store the new terminal control Index: telnetd/state.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /mnt/ncvs/src/crypto/telnet/telnetd/state.c,v retrieving revision 1.7 diff -u -r1.7 state.c --- telnetd/state.c 2001/07/19 18:58:31 1.7 +++ telnetd/state.c 2001/07/23 04:41:52 @@ -1615,40 +1615,69 @@ =20 /* * This function appends data to nfrontp and advances nfrontp. + * Returns the number of characters written altogether (the buffer may have + * been flushed in the process). */ =20 int output_data(const char *format, ...) { va_list args; - size_t remaining, ret; + size_t remaining, copied; + int bufremain; + char *buf, *bufp; =20 va_start(args, format); + /* calculate free space to play with */ remaining =3D BUFSIZ - (nfrontp - netobuf); - /* try a netflush() if the room is too low */ - if (strlen(format) > remaining || BUFSIZ / 4 > remaining) { - netflush(); - remaining =3D BUFSIZ - (nfrontp - netobuf); + + if ((bufremain =3D vasprintf(&buf, format, args)) =3D=3D -1) + return -1; + bufp =3D buf; + + while (bufremain > 0) { + /* Free up enough space if the room is too low */ + while ((bufremain > BUFSIZ ? BUFSIZ : bufremain) > remaining) + remaining +=3D netflush(); + + /* Copy out as much as will fit */ + copied =3D remaining > bufremain ? bufremain : remaining; + memmove(nfrontp, bufp, copied); + nfrontp +=3D copied; + bufremain -=3D copied; + remaining -=3D copied; + bufp +=3D copied; } - ret =3D vsnprintf(nfrontp, remaining, format, args); - nfrontp +=3D ((ret < remaining - 1) ? ret : remaining - 1); va_end(args); - return ret; + bufremain =3D strlen(buf); + free(buf); + return bufremain; } =20 int output_datalen(const char *buf, size_t len) { size_t remaining; - + int bufremain, copied; + const char *bufp; +=09 remaining =3D BUFSIZ - (nfrontp - netobuf); - if (remaining < len) { - netflush(); - remaining =3D BUFSIZ - (nfrontp - netobuf); + bufremain =3D len; + bufp =3D buf; + + while (bufremain > 0) { + /* Free up enough space if the room is too low*/ + while((bufremain > BUFSIZ ? BUFSIZ : bufremain) > remaining) + remaining +=3D netflush(); + + /* Copy out as much as will fit */ + copied =3D remaining > bufremain ? bufremain : remaining; + memmove(nfrontp, bufp, copied); + nfrontp +=3D copied; + bufremain -=3D copied; + remaining -=3D copied; + bufp +=3D copied; } - if (remaining < len) - return -1; - memmove(nfrontp, buf, len); - nfrontp +=3D len; + return (len); } Index: telnetd/telnetd.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /mnt/ncvs/src/crypto/telnet/telnetd/telnetd.c,v retrieving revision 1.16 diff -u -r1.16 telnetd.c --- telnetd/telnetd.c 2001/07/19 17:48:57 1.16 +++ telnetd/telnetd.c 2001/07/23 04:44:39 @@ -952,7 +952,6 @@ char *HE; char *HN; char *IM; - void netflush(); int nfd; =20 /* @@ -1420,8 +1419,9 @@ } #endif /* defined(CRAY2) && defined(UNICOS5) */ =20 - if (FD_ISSET(f, &obits) && (nfrontp - nbackp) > 0) - netflush(); + if (FD_ISSET(f, &obits)) + while ((nfrontp - nbackp) > 0) + netflush(); if (ncc > 0) telrcv(); if (FD_ISSET(p, &obits) && (pfrontp - pbackp) > 0) Index: telnetd/termstat.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /mnt/ncvs/src/crypto/telnet/telnetd/termstat.c,v retrieving revision 1.7 diff -u -r1.7 termstat.c --- telnetd/termstat.c 2001/07/19 17:48:57 1.7 +++ telnetd/termstat.c 2001/07/23 03:38:03 @@ -140,7 +140,6 @@ void localstat() { - void netflush(); int need_will_echo =3D 0; =20 #if defined(CRAY2) && defined(UNICOS5) @@ -404,7 +403,6 @@ clientstat(code, parm1, parm2) register int code, parm1, parm2; { - void netflush(); =20 /* * Get a copy of terminal characteristics. Index: telnetd/utility.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /mnt/ncvs/src/crypto/telnet/telnetd/utility.c,v retrieving revision 1.8 diff -u -r1.8 utility.c --- telnetd/utility.c 2001/07/19 17:48:57 1.8 +++ telnetd/utility.c 2001/07/23 04:48:14 @@ -69,10 +69,9 @@ void ttloop() { - void netflush(); =20 DIAG(TD_REPORT, output_data("td: ttloop\r\n")); - if (nfrontp-nbackp) { + while ((nfrontp - nbackp) > 0) { netflush(); } ncc =3D read(net, netibuf, sizeof netibuf); @@ -249,18 +248,22 @@ /* * netflush * Send as much data as possible to the network, - * handling requests for urgent data. + * handling requests for urgent data. Not all data in the + * buffer may be sent. */ - void + int netflush() { int n; extern int not42; =20 if ((n =3D nfrontp - nbackp) > 0) { +#if 0 + /* XXX This causes output_data() to recurse and die */ DIAG(TD_REPORT, { n +=3D output_data("td: netflush %d chars\r\n", n); }); +#endif #ifdef ENCRYPTION if (encrypt_output) { char *s =3D nclearto ? nclearto : nbackp; @@ -296,7 +299,7 @@ } if (n < 0) { if (errno =3D=3D EWOULDBLOCK || errno =3D=3D EINTR) - return; + return 0; cleanup(0); } nbackp +=3D n; @@ -313,7 +316,7 @@ nclearto =3D 0; #endif /* ENCRYPTION */ } - return; + return (n); } /* end of netflush */ =20 =20 @@ -1109,7 +1112,7 @@ =20 while (cnt) { /* flush net output buffer if no room for new data) */ - if ((&netobuf[BUFSIZ] - nfrontp) < 80) { + while ((&netobuf[BUFSIZ] - nfrontp) < 80) { netflush(); } =20 --KsGdsel6WgEHnImy Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.0.6 (FreeBSD) Comment: For info see http://www.gnupg.org iD8DBQE7W65zWry0BWjoQKURAmK0AJ4gC0PWDr+lvGopXUeg+dYjKYw6UACg6Hc1 simSHwuN/kZOaH+phyMBBow= =fa5T -----END PGP SIGNATURE----- --KsGdsel6WgEHnImy-- To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message