Am 30.10.2015 um 14:58 schrieb Steffan Karger:
Hi Holger, On Fri, Oct 30, 2015 at 2:34 PM, Holger Kummert <holger.kumm...@sophos.com> wrote:static void -add_security_buffer(int sb_offset, void *data, int length, unsigned char *msg_buf, int *msg_bufpos) +add_security_buffer(int sb_offset, void *data, int length, unsigned char *msg_buf, int *msg_bufpos, int msg_buflen) { + if ((*msg_bufpos - *msg_buf) + length > msg_buflen) { + msg (M_INFO, "Warning: Not enough space in destination buffer"); + return; + } +I don't think this is what you intended, or does *msg_buf contain some magic length field? You probably want something like: ASSERT (length > 0 && *msg_bufpos > 0 && msg_buflen > 0); if (msg_buflen - length < msg_bufpos) { /* error */ } That should be an integer-overflow safe overflow check.Hmm, I think it won't work as msg_bufpos is a pointer into memory, right? What about improving the first version with ASSERT (length > 0 && *msg_bufpos > 0 && msg_buflen > 0); if ((msg_bufpos - msg_buf) + length > msg_buflen) { ...Seems I forgot the * before the second msg_bufpos, sorry. msg_bufpos itself is a pointer indeed, but *msg_bufpos is (used as) an offset into msg_buf: memcpy(&msg_buf[*msg_bufpos], data, msg_buf[sb_offset]); msg_bufpos (without the *) is not related to msg_buf.
Ah, then your first proposal makes sense (i.e. if (msg_buflen - length < *msg_bufpos) ) We could take that. Sry, didn't look into the ntlm-code ... Regards, Holger
