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 "[EMAIL PROTECTED]"
