[FFmpeg-devel] [PATCH] avformat/hlsenc: hold old key info when append list
Aaron Levinson
alevinsn at aracnet.com
Fri May 5 07:29:18 EEST 2017
On 5/4/2017 9:15 PM, Steven Liu wrote:
> 2017-05-03 9:49 GMT+08:00 Aaron Levinson <alevinsn at aracnet.com>:
>
>> On 4/27/2017 7:21 PM, Steven Liu wrote:
>>
>>> 2017-04-26 7:30 GMT+08:00 Steven Liu <lq at chinaffmpeg.org>:
>>>
>>> fix ticket id: #6353
>>>>
>>>> Signed-off-by: Steven Liu <lq at chinaffmpeg.org>
>>>> ---
>>>> libavformat/hlsenc.c | 24 ++++++++++++++++++++++++
>>>> 1 file changed, 24 insertions(+)
>>>>
>>>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>>>> index 27c8e3355d..3ec0f330fb 100644
>>>> --- a/libavformat/hlsenc.c
>>>> +++ b/libavformat/hlsenc.c
>>>> @@ -810,6 +810,7 @@ static int parse_playlist(AVFormatContext *s, const
>>>> char *url)
>>>> int64_t new_start_pos;
>>>> char line[1024];
>>>> const char *ptr;
>>>> + const char *end;
>>>>
>>>> if ((ret = ffio_open_whitelist(&in, url, AVIO_FLAG_READ,
>>>> &s->interrupt_callback, NULL,
>>>> @@ -842,6 +843,29 @@ static int parse_playlist(AVFormatContext *s, const
>>>> char *url)
>>>> } else if (av_strstart(line, "#EXTINF:", &ptr)) {
>>>> is_segment = 1;
>>>> hls->duration = atof(ptr);
>>>> + } else if (av_stristart(line, "#EXT-X-KEY:", &ptr)) {
>>>> + ptr = av_stristr(line, "URI=\"");
>>>> + if (ptr) {
>>>> + ptr += strlen("URI=\"");
>>>> + end = av_stristr(ptr, ",");
>>>> + if (end) {
>>>> + av_strlcpy(hls->key_uri, ptr, end - ptr);
>>>> + } else {
>>>> + av_strlcpy(hls->key_uri, ptr, sizeof(hls->key_uri));
>>>>
>>>
>> I know that this patch has already been applied (although it didn't get
>> any reviews on the ML), but I think that there are some issues with it.
>> First, it seems that both av_strlcpy() cases above will copy the
>> terminating '\"' character into hls->key_uri. Perhaps that won't cause an
>> issue in practice, but it is likely not the intended result. Second, with
>> both av_strlcpy() calls that use a length of end - ptr, this could in
>> theory result in a buffer overrun depending on how long the URI is, since
>> the destination buffers have a fixed size. This issue is prevented in the
>> second call to av_strlcpy(), since it uses sizeof(hls->key_uri), but it is
>> a potential issue with the first calls (note that this comment applies to
>> the IV=0x code below). If you want to use av_strlcpy(), either make sure
>> that end - ptr is less than or equal to sizeof(hls->key_uri) or do
>> something like *end = 0 first and then use av_strlcpy(hls->key_uri, ptr,
>> sizeof(hls->key_uri)).
>>
>> In addition, based on the EXT-X-KEY example at
>> https://developer.apple.com/library/content/technotes/tn2288/_index.html
>> , it would appear that an EXT-X-KEY declaration may span multiple lines.
>> Your solution will not work properly in this case.
>>
> Hi Aaron,
> I think the libavformat/hls.c maybe have the same problem, because
> both of hlsenc and hls use read_chomp_line to read one line,
> that need check the '\' tail a line, and read the next line into a
> buffer, that maybe better, is that right?
Yes, I was thinking the same thing, that read_chomp_line() needs to be
enhanced to deal with declarations that span multiple lines. That
probably belongs in a separate patch though, even if it is only relevant
for EXT-X-KEY.
Aaron Levinson
More information about the ffmpeg-devel
mailing list