Date: Thu, 31 Mar 2005 17:56:53 -0500 From: Steve Kiernan <stevek@juniper.net> To: Colin Percival <cperciva@freebsd.org> Cc: freebsd-security@freebsd.org Subject: Re: FreeBSD Security Advisory FreeBSD-SA-05:01.telnet Message-ID: <1112309813.8421.96.camel@localhost> In-Reply-To: <424C7B88.9030605@freebsd.org> References: <1112296855.8421.64.camel@localhost> <424C7B88.9030605@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1112309813.8421.96.camel>