FreeBSD Security Advisory FreeBSD-SA-05:01.telnet
stevek at juniper.net
Thu Mar 31 14:57:04 PST 2005
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;
> > +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];
> > 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 > 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 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 == slc_reply_eom, but (looking at the code)
> the two final bytes will be written into slc_reply and
Ah, yes, you are correct, the tests are just odd. Thanks.
More information about the freebsd-security