[FFmpeg-devel] [PATCH] ffserver: fix incorrect strlcpy usage

[Ganesh Ajjanagadde]

On Fri, Nov 6, 2015 at 10:42 PM, Mark Harris <mark.hsj at gmail.com> wrote:
> On Fri, Nov 6, 2015 at 12:49 PM, Ganesh Ajjanagadde
> <gajjanagadde at gmail.com> wrote:
>> Somewhat ironic that this "safe" interface is actually being used
>> unsafely here. This fixes the usage preventing potential null pointer
>> dereference, where the old code was doubly broken: ctime can return
>> NULL, and ctime can return an arbitrarily long buffer.
>>
>> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>> ---
>>  ffserver.c | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/ffserver.c b/ffserver.c
>> index 526cbfc..108523e 100644
>> --- a/ffserver.c
>> +++ b/ffserver.c
>> @@ -305,15 +305,18 @@ static void ffm_set_write_index(AVFormatContext *s, int64_t pos,
>>      ffm->file_size = file_size;
>>  }
>>
>> -static char *ctime1(char *buf2, int buf_size)
>> +static char *ctime1(char *buf2, size_t buf_size)
>>  {
>>      time_t ti;
>>      char *p;
>>
>>      ti = time(NULL);
>>      p = ctime(&ti);
>> -    av_strlcpy(buf2, p, buf_size);
>> -    p = buf2 + strlen(p) - 1;
>> +    if (!p) {
>> +        *buf2 = '\0';
>> +        return buf2;
>> +    }
>> +    p = buf2 + av_strlcpy(buf2, p, buf_size) - 1;
>>      if (*p == '\n')
>>          *p = '\0';
>>      return buf2;
>
> Ironically, this still doesn't handle a string that is too long for
> the buffer.  "safe" indeed!  strlcpy (and av_strlcpy) returns the
> length of the source, not the destination, so this will still access
> and possibly write to memory beyond the end of buf2 when the string is
> truncated.

Just looked, yes, this interface is nasty. But then again all string
copying stuff is rather horrible.

>
> It will also still access and possibly write to one byte before the
> beginning of the buffer if the string is an empty string, although
> that "should not happen".

True, but while fixing things, they should be fixed correctly. Will
try again, thanks.