From owner-freebsd-security@FreeBSD.ORG  Thu Mar 31 22:57:03 2005
Return-Path: <owner-freebsd-security@FreeBSD.ORG>
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 E011216A4CE; Thu, 31 Mar 2005 22:57:03 +0000 (GMT)
Received: from kremlin.juniper.net (kremlin.juniper.net [207.17.137.120])
	by mx1.FreeBSD.org (Postfix) with ESMTP
	id B6B4143D1D; Thu, 31 Mar 2005 22:57:03 +0000 (GMT)
	(envelope-from stevek@juniper.net)
Received: from unknown (HELO beta.jnpr.net) (172.24.18.109)
  by kremlin.juniper.net with ESMTP; 31 Mar 2005 14:57:04 -0800
X-BrightmailFiltered: true
X-Brightmail-Tracker: AAAAAA==
X-IronPort-AV: i="3.91,139,1110182400"; 
   d="scan'208"; a="285244842:sNHT22134856"
Received: from stevek-bsd.jnpr.net ([172.25.41.27]) by beta.jnpr.net with
	Microsoft SMTPSVC(6.0.3790.211);	 Thu, 31 Mar 2005 14:57:03 -0800
From: Steve Kiernan <stevek@juniper.net>
To: Colin Percival <cperciva@freebsd.org>
In-Reply-To: <424C7B88.9030605@freebsd.org>
References: <1112296855.8421.64.camel@localhost>
	 <424C7B88.9030605@freebsd.org>
Content-Type: text/plain
Organization: Juniper Networks Inc.
Date: Thu, 31 Mar 2005 17:56:53 -0500
Message-Id: <1112309813.8421.96.camel@localhost>
Mime-Version: 1.0
X-Mailer: Evolution 2.0.2 FreeBSD GNOME Team Port 
Content-Transfer-Encoding: 7bit
X-OriginalArrivalTime: 31 Mar 2005 22:57:03.0364 (UTC)
	FILETIME=[F4431440:01C53644]
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]
	<freebsd-security.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-security>,
	<mailto:freebsd-security-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-security>
List-Post: <mailto:freebsd-security@freebsd.org>
List-Help: <mailto:freebsd-security-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-security>,
	<mailto:freebsd-security-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Thu, 31 Mar 2005 22:57:04 -0000

On Thu, 2005-03-31 at 14:36 -0800, Colin Percival wrote:
> 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].

Ah, yes, you are correct, the tests are just odd.  Thanks.

-- 
Steve Kiernan
Juniper Networks