Re: FreeBSD Security Advisory FreeBSD-SA-05:01.telnet

From: Colin Percival (cperciva_at_freebsd.org)
Date: 04/01/05

  • Next message: Steve Kiernan: "Re: FreeBSD Security Advisory FreeBSD-SA-05:01.telnet" 
    Date: Thu, 31 Mar 2005 14:36:56 -0800
To: Steve Kiernan <stevek@juniper.net>

    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
    _______________________________________________
    freebsd-security@freebsd.org mailing list
    http://lists.freebsd.org/mailman/listinfo/freebsd-security
    To unsubscribe, send any mail to "freebsd-security-unsubscribe@freebsd.org"

  • Next message: Steve Kiernan: "Re: FreeBSD Security Advisory FreeBSD-SA-05:01.telnet"