[FFmpeg-devel] avformat/hlsenc: Don't segfault on uncommon names

The parsing process of the AVOpt-enabled string controlling the mapping
of input streams to variant streams is roughly as follows: Space and tab
separate variant stream group maps while the entries in each variant
stream group map are separated by ','.

The parsing process of each variant stream group proceeded as follows:
At first the number of occurences of "a:", "v:" and "s:" in each variant
stream group is calculated so that one can can allocate an array of
streams with this number of entries. Then the string is split along ','
and each substring is parsed. If such a substring starts with "a:", "s:"
or "v:" it is treated as stream specifier and (if there is a correct
number after ':') a stream of the variant stream is mapped to one of the
actual input streams.

Nothing actually guarantees that the number of streams allocated initially
equals the number of streams that are mapped to an actual input stream.
These numbers can differ if e.g. the name, the sgroup, agroup or ccgroup
of the variant stream contain "a:", "s:" or "v:".

The problem hereby is that the rest of the code presumes these numbers
to be equal and segfaults if it isn't (because the corresponding input
stream is NULL).

This commit fixes this by modifying the initial counting process to only
count occurences of "a:", "s:" or "v:" that are at the beginning or that
immediately follow a ','.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
Alternatively, one could error out if these two counts differed (in
which case one can conclude that one of the other values must have
contained "a:", "s:" or "v:"). I have not done so, because using these
doesn't seem to be forbidden at all and there might even be usecases
(think of "name:The_Lord_of_the_Rings:_The_Two_Towers" or "Avengers:").

Furthermore modifying the check has the advantage of not allocating to
much and it also allows to introduce keys that end with 'a', 's' or 'v'.

 libavformat/hlsenc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Andreas Rheinhardt:
> The parsing process of the AVOpt-enabled string controlling the mapping
> of input streams to variant streams is roughly as follows: Space and tab
> separate variant stream group maps while the entries in each variant
> stream group map are separated by ','.
> 
> The parsing process of each variant stream group proceeded as follows:
> At first the number of occurences of "a:", "v:" and "s:" in each variant
> stream group is calculated so that one can can allocate an array of
> streams with this number of entries. Then the string is split along ','
> and each substring is parsed. If such a substring starts with "a:", "s:"
> or "v:" it is treated as stream specifier and (if there is a correct
> number after ':') a stream of the variant stream is mapped to one of the
> actual input streams.
> 
> Nothing actually guarantees that the number of streams allocated initially
> equals the number of streams that are mapped to an actual input stream.
> These numbers can differ if e.g. the name, the sgroup, agroup or ccgroup
> of the variant stream contain "a:", "s:" or "v:".
> 
> The problem hereby is that the rest of the code presumes these numbers
> to be equal and segfaults if it isn't (because the corresponding input
> stream is NULL).
> 
> This commit fixes this by modifying the initial counting process to only
> count occurences of "a:", "s:" or "v:" that are at the beginning or that
> immediately follow a ','.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
> Alternatively, one could error out if these two counts differed (in
> which case one can conclude that one of the other values must have
> contained "a:", "s:" or "v:"). I have not done so, because using these
> doesn't seem to be forbidden at all and there might even be usecases
> (think of "name:The_Lord_of_the_Rings:_The_Two_Towers" or "Avengers:").
> 
> Furthermore modifying the check has the advantage of not allocating to
> much and it also allows to introduce keys that end with 'a', 's' or 'v'.
> 
>  libavformat/hlsenc.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index 5695c6cc95..a381ca3e9e 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -1951,10 +1951,13 @@ static int parse_variant_stream_mapstring(AVFormatContext *s)
>              return AVERROR(EINVAL);
>  
>          q = varstr;
> -        while (q < varstr + strlen(varstr)) {
> +        while (1) {
>              if (!av_strncasecmp(q, "a:", 2) || !av_strncasecmp(q, "v:", 2) ||
>                  !av_strncasecmp(q, "s:", 2))
>                  vs->nb_streams++;
> +            q = strchr(q, ',');
> +            if (!q)
> +                break;
>              q++;
>          }
>          vs->streams = av_mallocz(sizeof(AVStream *) * vs->nb_streams);
> 
Will apply this tomorrow if there are no objections.

- Andreas

Andreas Rheinhardt:
> Andreas Rheinhardt:
>> The parsing process of the AVOpt-enabled string controlling the mapping
>> of input streams to variant streams is roughly as follows: Space and tab
>> separate variant stream group maps while the entries in each variant
>> stream group map are separated by ','.
>>
>> The parsing process of each variant stream group proceeded as follows:
>> At first the number of occurences of "a:", "v:" and "s:" in each variant
>> stream group is calculated so that one can can allocate an array of
>> streams with this number of entries. Then the string is split along ','
>> and each substring is parsed. If such a substring starts with "a:", "s:"
>> or "v:" it is treated as stream specifier and (if there is a correct
>> number after ':') a stream of the variant stream is mapped to one of the
>> actual input streams.
>>
>> Nothing actually guarantees that the number of streams allocated initially
>> equals the number of streams that are mapped to an actual input stream.
>> These numbers can differ if e.g. the name, the sgroup, agroup or ccgroup
>> of the variant stream contain "a:", "s:" or "v:".
>>
>> The problem hereby is that the rest of the code presumes these numbers
>> to be equal and segfaults if it isn't (because the corresponding input
>> stream is NULL).
>>
>> This commit fixes this by modifying the initial counting process to only
>> count occurences of "a:", "s:" or "v:" that are at the beginning or that
>> immediately follow a ','.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
>> ---
>> Alternatively, one could error out if these two counts differed (in
>> which case one can conclude that one of the other values must have
>> contained "a:", "s:" or "v:"). I have not done so, because using these
>> doesn't seem to be forbidden at all and there might even be usecases
>> (think of "name:The_Lord_of_the_Rings:_The_Two_Towers" or "Avengers:").
>>
>> Furthermore modifying the check has the advantage of not allocating to
>> much and it also allows to introduce keys that end with 'a', 's' or 'v'.
>>
>>  libavformat/hlsenc.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
>> index 5695c6cc95..a381ca3e9e 100644
>> --- a/libavformat/hlsenc.c
>> +++ b/libavformat/hlsenc.c
>> @@ -1951,10 +1951,13 @@ static int parse_variant_stream_mapstring(AVFormatContext *s)
>>              return AVERROR(EINVAL);
>>  
>>          q = varstr;
>> -        while (q < varstr + strlen(varstr)) {
>> +        while (1) {
>>              if (!av_strncasecmp(q, "a:", 2) || !av_strncasecmp(q, "v:", 2) ||
>>                  !av_strncasecmp(q, "s:", 2))
>>                  vs->nb_streams++;
>> +            q = strchr(q, ',');
>> +            if (!q)
>> +                break;
>>              q++;
>>          }
>>          vs->streams = av_mallocz(sizeof(AVStream *) * vs->nb_streams);
>>
> Will apply this tomorrow if there are no objections.
> 
> - Andreas
> 
Applied.

- Andreas