From owner-freebsd-security@FreeBSD.ORG Thu Mar 31 22:37:11 2005 Return-Path: Delivered-To: freebsd-security@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 4059216A4CE for ; Thu, 31 Mar 2005 22:37:11 +0000 (GMT) Received: from pd3mo3so.prod.shaw.ca (shawidc-mo1.cg.shawcable.net [24.71.223.10]) by mx1.FreeBSD.org (Postfix) with ESMTP id DFA8B43D39 for ; Thu, 31 Mar 2005 22:37:10 +0000 (GMT) (envelope-from cperciva@freebsd.org) Received: from pd5mr7so.prod.shaw.ca (pd5mr7so-qfe3.prod.shaw.ca [10.0.141.183]) by l-daemon (Sun ONE Messaging Server 6.0 HotFix 1.01 (built Mar 15 2004)) with ESMTP id <0IE800BNTMTVAGJK@l-daemon> for freebsd-security@freebsd.org; Thu, 31 Mar 2005 15:37:07 -0700 (MST) Received: from pn2ml1so.prod.shaw.ca ([10.0.121.145]) by pd5mr7so.prod.shaw.ca (Sun ONE Messaging Server 6.0 HotFix 1.01 (built Mar 15 2004)) with ESMTP id <0IE8002V5MTV1RI0@pd5mr7so.prod.shaw.ca> for freebsd-security@freebsd.org; Thu, 31 Mar 2005 15:37:07 -0700 (MST) Received: from [192.168.0.60] (S0106006067227a4a.vc.shawcable.net [24.87.209.6]) by l-daemon (iPlanet Messaging Server 5.2 HotFix 1.18 (built Jul 28 2003)) freebsd-security@freebsd.org; Thu, 31 Mar 2005 15:37:07 -0700 (MST) Date: Thu, 31 Mar 2005 14:36:56 -0800 From: Colin Percival In-reply-to: <1112296855.8421.64.camel@localhost> To: Steve Kiernan Message-id: <424C7B88.9030605@freebsd.org> MIME-version: 1.0 Content-type: text/plain; charset=ISO-8859-1 Content-transfer-encoding: 7bit X-Accept-Language: en-us, en X-Enigmail-Version: 0.90.1.0 X-Enigmail-Supports: pgp-inline, pgp-mime References: <1112296855.8421.64.camel@localhost> User-Agent: Mozilla Thunderbird 1.0.2 (X11/20050326) cc: freebsd-security@freebsd.org Subject: Re: FreeBSD Security Advisory FreeBSD-SA-05:01.telnet X-BeenThere: freebsd-security@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Security issues [members-only posting] List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 31 Mar 2005 22:37:11 -0000 Steve Kiernan wrote: > I was looking at this patch, but there seems to be an error in it: > > unsigned char slc_reply[128]; > +unsigned char const * const slc_reply_eom = &slc_reply[sizeof(slc_reply)]; > unsigned char *slc_replyp; > > Should the value for slc_reply_eom not be this instead? > > unsigned char const * const slc_reply_eom = &slc_reply[sizeof(slc_reply) - 1]; No. > Considering the conditionals are the following: > > + if (&slc_replyp[6+2] > slc_reply_eom) > + return; > > .. and .. > > + /* The end of negotiation command requires 2 bytes. */ > + if (&slc_replyp[2] > slc_reply_eom) > + return; > > If you don't subtract 1 from the sizeof(slc_reply) or change the > conditional operators to >=, then you could try to write one byte past > the end of the buffer. The tests are written a bit oddly, but I'm fairly certain that they are correct. &slc_replyp[6+2] and &slc_replyp[2] are not the addresses of the last bytes which will be written; rather, they are the addresses of the byte after the last byte which will be written. Taking the second example, if slc_replyp == slc_reply + 126, then we will have &slc_replyp[2] == slc_reply_eom, but (looking at the code) the two final bytes will be written into slc_reply[126] and slc_reply[127]. Colin Percival