Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 20 Jul 2001 10:00:29 +0300
From:      Ruslan Ermilov <ru@FreeBSD.ORG>
To:        Matt Dillon <dillon@earth.backplane.com>
Cc:        Assar Westerlund <assar@FreeBSD.ORG>, security@FreeBSD.ORG
Subject:   Re: [PATCH] Re: FreeBSD remote root exploit ?
Message-ID:  <20010720100029.A30828@sunbay.com>
In-Reply-To: <200107191917.f6JJHwV77405@earth.backplane.com>; from dillon@earth.backplane.com on Thu, Jul 19, 2001 at 12:17:58PM -0700
References:  <5.1.0.14.0.20010719001357.03e22638@192.168.0.12> <014d01c11031$bdab5a10$2001a8c0@clitoris> <20010719201407.B61061@sunbay.com> <003701c11077$b3125400$0d00a8c0@alexus> <3B5718A0.2B650C9C@oksala.org> <200107191752.f6JHqer75736@earth.backplane.com> <20010719205948.D67829@sunbay.com> <200107191817.f6JIHSJ76262@earth.backplane.com> <20010719215957.A74024@sunbay.com> <200107191917.f6JJHwV77405@earth.backplane.com>

next in thread | previous in thread | raw e-mail | index | archive | help

--Dxnq1zWXvFF0Q93v
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Thu, Jul 19, 2001 at 12:17:58PM -0700, Matt Dillon wrote:
> 
> :> 
> :>     The output_data() calls for the various options are safe, strlen(format)
> :>     will always be larger then the actual formatted result.  But the 
> :>     debugging and crypto calls to output_data() are not safe.
> :> 
> :> 						-Matt
> :> 
> :> :                netflush();
> :> :                remaining = BUFSIZ - (nfrontp - netobuf);
> :> :        }
> :> :        ret = vsnprintf(nfrontp, remaining, format, args);
> :> 
> :Should be fixed in state.c,v 1.7.  Thanks, Assar!
> :
> :
> :Cheers,
> :-- 
> :Ruslan Ermilov		Oracle Developer/DBA,
> :ru@sunbay.com		Sunbay Software AG,
> 
>     heh heh.  Sorry guys, state.c still isn't quite right.
> 
>     nfrontp += ((ret < remaining - 1) ? ret : remaining - 1);
> 
>     What happens if remaining is 0 ?
> 
Umm, let's count.

0.  Let netobuf be 0.
1.  nfrontp = BUFSIZ - 3
2.  remaining = 3 and we try to write 10 bytes.
3.  ret = 10
4.  (10 < 3 - 1) ? 10 : 3 - 1 = 2
5.  nfrontp += 2 = BUFSIZ - 1

[next 10 bytes write]

6.  remaining = BUFSIZ - (BUFSIZ - 1) = 1
7.  ret = 10
8.  (10 < 1 - 1) ? 10 : 1 - 1 = 0
9.  nfrontp += 0 = BUFSIZ - 1

remaining = BUFSIZ - (nfrontp - netobuf) = BUFSIZ - ((BUFSIZ - 1) - 0) = 1

So, the minimum possible value for `remaining' is 1.

OTOH, we have another routine that advances nfrontp():

: int
: output_datalen(const char *buf, size_t len)
: {
: 	size_t remaining;
: 
: 	remaining = BUFSIZ - (nfrontp - netobuf);
: 	if (remaining < len) {
: 		netflush();
: 		remaining = BUFSIZ - (nfrontp - netobuf);
: 	}
: 	if (remaining < len)
: 		return -1;
: 	memmove(nfrontp, buf, len);
: 	nfrontp += len;
: 	return (len);
: }

1.  nfrontp = BUFSIZ - 3
2.  remaining = 3 and we write len = 3 bytes.
3.  nfrontp += 3 = BUFSIZ

Then, on the next call to output_data()

4.  remaining = 0
and, assuming that netflush() did nothing(!)
5.  ret = 10 (10 bytes write attempt)
6.  (10 < 0 - 1) ? 10 : 0 - 1 = -1
7.  nfrontp += -1 = nfrontp - 1

So, the worst we can have `nfrontp' decremented by one.
Not overflowable, but not right.

OK, how about the following?

It should be OK if `nfrontp' points beyond one byte of
`netobuf'.  See netflush() for details.

Please review.


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

--Dxnq1zWXvFF0Q93v
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.5
diff -u -p -r1.5 ext.h
--- ext.h	2001/07/19 17:48:57	1.5
+++ ext.h	2001/07/20 06:50:28
@@ -74,7 +74,7 @@ extern char	ptyobuf[BUFSIZ+NETSLOP], *pf
 
 extern char	netibuf[BUFSIZ], *netip;
 
-extern char	netobuf[BUFSIZ+NETSLOP], *nfrontp, *nbackp;
+extern char	netobuf[BUFSIZ], *nfrontp, *nbackp;
 extern char	*neturg;		/* one past last bye of urgent data */
 
 extern int	pcc, ncc;
Index: state.c
===================================================================
RCS file: /home/ncvs/src/crypto/telnet/telnetd/state.c,v
retrieving revision 1.7
diff -u -p -r1.7 state.c
--- state.c	2001/07/19 18:58:31	1.7
+++ state.c	2001/07/20 06:51:13
@@ -1631,7 +1631,7 @@ output_data(const char *format, ...)
 		remaining = BUFSIZ - (nfrontp - netobuf);
 	}
 	ret = vsnprintf(nfrontp, remaining, format, args);
-	nfrontp += ((ret < remaining - 1) ? ret : remaining - 1);
+	nfrontp += (ret < remaining) ? ret : remaining;
 	va_end(args);
 	return ret;
 }

--Dxnq1zWXvFF0Q93v--

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-security" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010720100029.A30828>