From owner-freebsd-security Fri Jul 20 2:33: 2 2001 Delivered-To: freebsd-security@freebsd.org Received: from earth.backplane.com (earth-nat-cw.backplane.com [208.161.114.67]) by hub.freebsd.org (Postfix) with ESMTP id 5685F37B407; Fri, 20 Jul 2001 02:32:43 -0700 (PDT) (envelope-from dillon@earth.backplane.com) Received: (from dillon@localhost) by earth.backplane.com (8.11.4/8.11.2) id f6K9WgZ88552; Fri, 20 Jul 2001 02:32:42 -0700 (PDT) (envelope-from dillon) Date: Fri, 20 Jul 2001 02:32:42 -0700 (PDT) From: Matt Dillon Message-Id: <200107200932.f6K9WgZ88552@earth.backplane.com> To: Ruslan Ermilov Cc: Assar Westerlund , security@FreeBSD.ORG Subject: Re: [PATCH] Re: FreeBSD remote root exploit ? 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> <20010720100029.A30828@sunbay.com> Sender: owner-freebsd-security@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org :.. : :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. Except on the NEXT call remaining will be negative, but since remaining is unsigned it will appear to be a very large number and the routine will believe that *any* length is legal. Now, of course, the attempt to netflush() will probably hide this potential problem but it is still a good idea to write bullet proof code that does not rely on caller assumptions. :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, The below fix seems reasonable. Strictly speaking it isn't really pointing beyond the end of netobuf, the pointer will simply be such that the length calculation will wind up being exactly the size of netobuf which is what you want. I would go further and just use 'int' instead of size_t in this routine, and to doubly guarentee that no miscalculation will occur you would assert() that remaining is >= 0 (in addition to the changes you make below). It pays to write safe code. -Matt : 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