10-fix_potential_overflows.dpatch

[Sven Müller]

Am 10.07.2010 16:09, schrieb Ondřej Surý:
> Ok, then just use memcpy instead of strncpy and it should be fine on both a you'll save some cycles by not checking for NULL char.

1) As said, I have personally not seen such a system with non-standard
strncpy.
2) memcpy wouldn't work because you also have to check for strings
shorter than "len" IIRC. Though the code snippet below looks like memcpy
might also work.

regards,
Sven

> On 10.7.2010, at 11:30, Sven Müller <sven at incase.de> wrote:
> 
>> Am 10.07.2010 00:36, schrieb Ondřej Surý:
>>
>> If I remember correctly here, the problem was that sometimes the copied
>> string was unneededly shortened by one character. As far as I recall,
>> there have been strncpy implementations that actually ensure the copied
>> string to be null terminated (though I have never seen such an
>> implementation on Debian or BSD systems).
>>
>> Apart from that, you are right: the len+1 character will always be
>> overwritten with \r.
>>
>> THe code in patched and in unpatched version should be functionally
>> equivalent with any standard-conforming strncpy implementation (which
>> does not make sure the copied string is null terminated).
>>
>> Regards,
>> Sven
>>
>>> I tend to agree with you. It's not wrong per se, but it's useless,
>>> since len+1 char will be always overwritten by '\r'. And if hdr is
>>> 0-terminated string there will be always \0 at len+1 char.
>>>
>>> Ondrej
>>>
>>> On Fri, Jul 9, 2010 at 22:58, Patrick Goetz <pgoetz at mail.utexas.edu> wrote:
>>>> Can someone, maybe Sven, take a look at this patch?  This can't possibly be
>>>> right:
>>>>
>>>> --- git~/imap/message.c 2010-01-16 19:22:57.000000000 -0200
>>>> +++ git/imap/message.c  2010-01-16 19:27:30.915091898 -0200
>>>> @@ -996,7 +996,7 @@
>>>>    /* Save header value */
>>>>    len = hdrend - hdr;
>>>>    message_ibuf_ensure(ibuf, len+2);
>>>> -    strncpy(ibuf->end, hdr, len);
>>>> +    strncpy(ibuf->end, hdr, len+1);
>>>>    ibuf->end += len;
>>>>    *(ibuf->end)++ = '\r';
>>>>    *(ibuf->end)++ = '\n';
>>>>
>>>>
>>>> _______________________________________________
>>>> Pkg-Cyrus-imapd-Debian-devel mailing list
>>>> Pkg-Cyrus-imapd-Debian-devel at lists.alioth.debian.org
>>>> http://lists.alioth.debian.org/mailman/listinfo/pkg-cyrus-imapd-debian-devel
>>>>
>>>
>>>
>>>
>>
>>
>> _______________________________________________
>> Pkg-Cyrus-imapd-Debian-devel mailing list
>> Pkg-Cyrus-imapd-Debian-devel at lists.alioth.debian.org
>> http://lists.alioth.debian.org/mailman/listinfo/pkg-cyrus-imapd-debian-devel
> 
> _______________________________________________
> Pkg-Cyrus-imapd-Debian-devel mailing list
> Pkg-Cyrus-imapd-Debian-devel at lists.alioth.debian.org
> http://lists.alioth.debian.org/mailman/listinfo/pkg-cyrus-imapd-debian-devel