Date: Fri, 20 Jul 2001 14:37:42 +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: <20010720143742.E65677@sunbay.com> In-Reply-To: <200107200932.f6K9WgZ88552@earth.backplane.com>; from dillon@earth.backplane.com on Fri, Jul 20, 2001 at 02:32:42AM -0700 References: <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> <200107200932.f6K9WgZ88552@earth.backplane.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Jul 20, 2001 at 02:32:42AM -0700, Matt Dillon wrote: > > :.. > : > :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. > Not taking into account the signedness of `remaining', how it could be negative? remaining = BUFSIZ - (nfrontp - netobuf); For `remaining' to be negative, the following must be true: nfrontp > netobuf + BUFSIZ But that's not possible. > :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. > Yes, just a simple "write" pointer of the FIFO queue. > 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. > I don't think this is now required as all `nfrontp' modifications have been fold into output_data(), output_datalen(), netflush(), and netclear(), and they appear to be safe. I have found yet one unsafe place, writenet(), which I have replaced with output_datalen(). 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 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?20010720143742.E65677>