From owner-freebsd-audit Mon Jul 23 3:38:59 2001 Delivered-To: freebsd-audit@freebsd.org Received: from whale.sunbay.crimea.ua (whale.sunbay.crimea.ua [212.110.138.65]) by hub.freebsd.org (Postfix) with ESMTP id 5EA0F37B403; Mon, 23 Jul 2001 03:38:11 -0700 (PDT) (envelope-from ru@whale.sunbay.crimea.ua) Received: (from ru@localhost) by whale.sunbay.crimea.ua (8.11.2/8.11.2) id f6NAa9889482; Mon, 23 Jul 2001 13:36:09 +0300 (EEST) (envelope-from ru) Date: Mon, 23 Jul 2001 13:36:09 +0300 From: Ruslan Ermilov To: Kris Kennaway Cc: Assar Westerlund , Matt Dillon , audit@FreeBSD.ORG Subject: Re: [PATCH] Re: FreeBSD remote root exploit ? Message-ID: <20010723133609.A88343@sunbay.com> References: <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> <20010722215619.A94874@xor.obsecurity.org> <20010722221413.A95414@xor.obsecurity.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="k+w/mQv8wyuph6w0" Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: <20010722221413.A95414@xor.obsecurity.org>; from kris@obsecurity.org on Sun, Jul 22, 2001 at 10:14:14PM -0700 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 --k+w/mQv8wyuph6w0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Sun, Jul 22, 2001 at 10:14:14PM -0700, Kris Kennaway wrote: > On Sun, Jul 22, 2001 at 09:56:20PM -0700, Kris Kennaway wrote: > > > * 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. > > After thinking about this a bit further, I've decided to make the > standalone netflush() calls flush the entire buffer. This seems like > a better thing to do than to potentially do nothing at all. > A yet better thing is to make netflush() flush the entire buffer. > I'm also not sure if I diffed the last patch against the correct CVS > versions. Updated patch follows. > > Index: state.c > =================================================================== > RCS file: /mnt/ncvs/src/crypto/telnet/telnetd/state.c,v > retrieving revision 1.4.2.1 > diff -u -r1.4.2.1 state.c > --- state.c 2001/07/20 15:16:52 1.4.2.1 > +++ state.c 2001/07/23 05:11:24 [...] > /* > * 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). > */ > > int > output_data(const char *format, ...) > { > va_list args; > - size_t remaining, ret; > + size_t remaining, copied; > + int bufremain; > + char *buf, *bufp; > > va_start(args, format); > + /* calculate free space to play with */ > remaining = BUFSIZ - (nfrontp - netobuf); > - /* try a netflush() if the room is too low */ > - if (strlen(format) > remaining || BUFSIZ / 4 > remaining) { > - netflush(); > - remaining = BUFSIZ - (nfrontp - netobuf); > + > + if ((bufremain = vasprintf(&buf, format, args)) == -1) > + return -1; > + bufp = buf; > + > + while (bufremain > 0) { > + /* Free up enough space if the room is too low */ > + while ((bufremain > BUFSIZ ? BUFSIZ : bufremain) > remaining) > + remaining += netflush(); > + > + /* Copy out as much as will fit */ > + copied = remaining > bufremain ? bufremain : remaining; > + memmove(nfrontp, bufp, copied); > + nfrontp += copied; > + bufremain -= copied; > + remaining -= copied; > + bufp += copied; > } > - ret = vsnprintf(nfrontp, remaining, format, args); > - nfrontp += (ret < remaining) ? ret : remaining; > va_end(args); > - return ret; > + bufremain = strlen(buf); ^^^^^^^^^^^ This is bogus, `buf' may contain zero bytes [ output_data("%c", '\0') ]. > + free(buf); > + return bufremain; > } > > int > output_datalen(const char *buf, size_t len) > { [...] > return (len); > } > output_datalen() always returns `len', make it `void'? How about the attached? It mostly the same, except: - netflush() now flushes the entire buffer rather than callers ensure this. We should probably sleep until `net' is ready for output using select(2) but I have not found code that would set non-blocking I/O mode on the socket. - output_data() uses output_datalen() internally to avoid unnecessary duplication of the code. - output_datalen() simplified a bit. Cheers, -- Ruslan Ermilov Oracle Developer/DBA, ru@sunbay.com Sunbay Software AG, ru@FreeBSD.org FreeBSD committer, +380.652.512.251 Simferopol, Ukraine http://www.FreeBSD.org The Power To Serve http://www.oracle.com Enabling The Information Age --k+w/mQv8wyuph6w0 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=p Index: ext.h =================================================================== RCS file: /home/ncvs/src/crypto/telnet/telnetd/ext.h,v retrieving revision 1.2.8.2 diff -u -p -r1.2.8.2 ext.h --- ext.h 2001/07/20 15:16:52 1.2.8.2 +++ ext.h 2001/07/23 10:13:05 @@ -190,7 +190,7 @@ extern void wontoption P((int)); int output_data __P((const char *, ...)) __printflike(1, 2); -int output_datalen __P((const char *, size_t)); +void output_datalen __P((const char *, int)); #ifdef ENCRYPTION extern void (*encrypt_output) P((unsigned char *, int)); Index: slc.c =================================================================== RCS file: /home/ncvs/src/crypto/telnet/telnetd/slc.c,v retrieving revision 1.4.2.1 diff -u -p -r1.4.2.1 slc.c --- slc.c 2001/07/20 15:16:52 1.4.2.1 +++ slc.c 2001/07/23 10:13:06 @@ -176,7 +176,6 @@ end_slc(bufp) register unsigned char **bufp; { register int len; - void netflush(); /* * If a change has occured, store the new terminal control Index: state.c =================================================================== RCS file: /home/ncvs/src/crypto/telnet/telnetd/state.c,v retrieving revision 1.4.2.1 diff -u -p -r1.4.2.1 state.c --- state.c 2001/07/20 15:16:52 1.4.2.1 +++ state.c 2001/07/23 10:13:13 @@ -1615,40 +1615,46 @@ send_status() /* * 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). */ int output_data(const char *format, ...) { va_list args; - size_t remaining, ret; + int len; + char *buf; va_start(args, format); - remaining = BUFSIZ - (nfrontp - netobuf); - /* try a netflush() if the room is too low */ - if (strlen(format) > remaining || BUFSIZ / 4 > remaining) { - netflush(); - remaining = BUFSIZ - (nfrontp - netobuf); - } - ret = vsnprintf(nfrontp, remaining, format, args); - nfrontp += (ret < remaining) ? ret : remaining; + if ((len = vasprintf(&buf, format, args)) == -1) + return -1; + output_datalen(buf, len); va_end(args); - return ret; + free(buf); + return (len); } -int -output_datalen(const char *buf, size_t len) +void +output_datalen(const char *buf, int len) { - size_t remaining; - + int remaining, copied; + remaining = BUFSIZ - (nfrontp - netobuf); - if (remaining < len) { - netflush(); - remaining = BUFSIZ - (nfrontp - netobuf); - if (remaining < len) - return -1; + while (len > 0) { + /* Free up enough space if the room is too low*/ + if ((len > BUFSIZ ? BUFSIZ : len) > remaining) { + netflush(); + remaining = BUFSIZ - (nfrontp - netobuf); + } + + /* Copy out as much as will fit */ + copied = remaining > len ? len : remaining; + memmove(nfrontp, buf, copied); + nfrontp += copied; + len -= copied; + remaining -= copied; + buf += copied; } - memmove(nfrontp, buf, len); - nfrontp += len; - return (len); + return; } Index: telnetd.c =================================================================== RCS file: /home/ncvs/src/crypto/telnet/telnetd/telnetd.c,v retrieving revision 1.11.2.3 diff -u -p -r1.11.2.3 telnetd.c --- telnetd.c 2001/07/20 15:16:52 1.11.2.3 +++ telnetd.c 2001/07/23 10:13:26 @@ -952,7 +952,6 @@ telnet(f, p, host) char *HE; char *HN; char *IM; - void netflush(); int nfd; /* Index: termstat.c =================================================================== RCS file: /home/ncvs/src/crypto/telnet/telnetd/termstat.c,v retrieving revision 1.4.2.2 diff -u -p -r1.4.2.2 termstat.c --- termstat.c 2001/07/20 15:16:52 1.4.2.2 +++ termstat.c 2001/07/23 10:13:26 @@ -140,7 +140,6 @@ int newmap = 1; /* nonzero if \n maps to void localstat() { - void netflush(); int need_will_echo = 0; #if defined(CRAY2) && defined(UNICOS5) @@ -404,7 +403,6 @@ flowstat() clientstat(code, parm1, parm2) register int code, parm1, parm2; { - void netflush(); /* * Get a copy of terminal characteristics. Index: utility.c =================================================================== RCS file: /home/ncvs/src/crypto/telnet/telnetd/utility.c,v retrieving revision 1.5.2.2 diff -u -p -r1.5.2.2 utility.c --- utility.c 2001/07/20 15:16:52 1.5.2.2 +++ utility.c 2001/07/23 10:13:26 @@ -69,10 +69,9 @@ static const char rcsid[] = void ttloop() { - void netflush(); DIAG(TD_REPORT, output_data("td: ttloop\r\n")); - if (nfrontp-nbackp) { + if (nfrontp - nbackp > 0) { netflush(); } ncc = read(net, netibuf, sizeof netibuf); @@ -257,10 +256,13 @@ netflush() int n; extern int not42; - if ((n = nfrontp - nbackp) > 0) { + while ((n = nfrontp - nbackp) > 0) { +#if 0 + /* XXX This causes output_data() to recurse and die */ DIAG(TD_REPORT, { n += output_data("td: netflush %d chars\r\n", n); }); +#endif #ifdef ENCRYPTION if (encrypt_output) { char *s = nclearto ? nclearto : nbackp; @@ -293,25 +295,24 @@ netflush() n = send(net, nbackp, n, MSG_OOB); /* URGENT data */ } } - } - if (n < 0) { - if (errno == EWOULDBLOCK || errno == EINTR) - return; - cleanup(0); - } - nbackp += n; + if (n == -1 && errno != EWOULDBLOCK && errno != EINTR) { + cleanup(0); + /* NOTREACHED */ + } + nbackp += n; #ifdef ENCRYPTION - if (nbackp > nclearto) - nclearto = 0; + if (nbackp > nclearto) + nclearto = 0; #endif /* ENCRYPTION */ - if (nbackp >= neturg) { - neturg = 0; - } - if (nbackp == nfrontp) { - nbackp = nfrontp = netobuf; + if (nbackp >= neturg) { + neturg = 0; + } + if (nbackp == nfrontp) { + nbackp = nfrontp = netobuf; #ifdef ENCRYPTION - nclearto = 0; + nclearto = 0; #endif /* ENCRYPTION */ + } } return; } /* end of netflush */ --k+w/mQv8wyuph6w0-- To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message