Date: Sun, 22 Jul 2001 21:56:20 -0700 From: Kris Kennaway <kris@obsecurity.org> To: Assar Westerlund <assar@FreeBSD.ORG> Cc: Kris Kennaway <kris@obsecurity.org>, Matt Dillon <dillon@earth.backplane.com>, Ruslan Ermilov <ru@FreeBSD.ORG>, audit@FreeBSD.org Subject: Re: [PATCH] Re: FreeBSD remote root exploit ? Message-ID: <20010722215619.A94874@xor.obsecurity.org> In-Reply-To: <5l66ck9wm7.fsf@assaris.sics.se>; from assar@FreeBSD.ORG on Mon, Jul 23, 2001 at 05:01:52AM %2B0200 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>
next in thread | previous in thread | raw e-mail | index | archive | help
--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 <kris@obsecurity.org> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010722215619.A94874>