diff mbox

[FFmpeg-devel,5/5] avformat/mpeg: Don't use unintialized value

Message ID 20191022131645.8394-5-andreas.rheinhardt@gmail.com
State New
Headers show

Commit Message

Andreas Rheinhardt Oct. 22, 2019, 1:16 p.m. UTC
vobsub_read_packet() didn't check whether an index in array of AVPackets
was valid and therefore used uninitialized values.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
Actually I only wanted to use Valgrind to check for memleaks...

 libavformat/mpeg.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Andreas Rheinhardt Jan. 18, 2020, 6:45 p.m. UTC | #1
On Tue, Oct 22, 2019 at 3:17 PM Andreas Rheinhardt <
andreas.rheinhardt@gmail.com> wrote:

> vobsub_read_packet() didn't check whether an index in array of AVPackets
> was valid and therefore used uninitialized values.
>
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
> Actually I only wanted to use Valgrind to check for memleaks...
>
>  libavformat/mpeg.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/libavformat/mpeg.c b/libavformat/mpeg.c
> index 73ade71d95..474afd06b9 100644
> --- a/libavformat/mpeg.c
> +++ b/libavformat/mpeg.c
> @@ -930,6 +930,10 @@ static int vobsub_read_packet(AVFormatContext *s,
> AVPacket *pkt)
>          FFDemuxSubtitlesQueue *tmpq = &vobsub->q[i];
>          int64_t ts;
>          av_assert0(tmpq->nb_subs);
> +
> +        if (tmpq->current_sub_idx >= tmpq->nb_subs)
> +            continue;
> +
>          ts = tmpq->subs[tmpq->current_sub_idx].pts;
>          if (ts < min_ts) {
>              min_ts = ts;
> --
> 2.20.1
>
>
Ping.

- Andreas
Michael Niedermayer Jan. 19, 2020, 11:56 a.m. UTC | #2
On Tue, Oct 22, 2019 at 03:16:45PM +0200, Andreas Rheinhardt wrote:
> vobsub_read_packet() didn't check whether an index in array of AVPackets
> was valid and therefore used uninitialized values.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
> Actually I only wanted to use Valgrind to check for memleaks...
> 
>  libavformat/mpeg.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/libavformat/mpeg.c b/libavformat/mpeg.c
> index 73ade71d95..474afd06b9 100644
> --- a/libavformat/mpeg.c
> +++ b/libavformat/mpeg.c
> @@ -930,6 +930,10 @@ static int vobsub_read_packet(AVFormatContext *s, AVPacket *pkt)
>          FFDemuxSubtitlesQueue *tmpq = &vobsub->q[i];
>          int64_t ts;
>          av_assert0(tmpq->nb_subs);
> +
> +        if (tmpq->current_sub_idx >= tmpq->nb_subs)
> +            continue;

How can this issue be reproduced ?

thx

[...]
Andreas Rheinhardt Jan. 19, 2020, 2:43 p.m. UTC | #3
Michael Niedermayer:
> On Tue, Oct 22, 2019 at 03:16:45PM +0200, Andreas Rheinhardt wrote:
>> vobsub_read_packet() didn't check whether an index in array of AVPackets
>> was valid and therefore used uninitialized values.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
>> ---
>> Actually I only wanted to use Valgrind to check for memleaks...
>>
>>  libavformat/mpeg.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/libavformat/mpeg.c b/libavformat/mpeg.c
>> index 73ade71d95..474afd06b9 100644
>> --- a/libavformat/mpeg.c
>> +++ b/libavformat/mpeg.c
>> @@ -930,6 +930,10 @@ static int vobsub_read_packet(AVFormatContext *s, AVPacket *pkt)
>>          FFDemuxSubtitlesQueue *tmpq = &vobsub->q[i];
>>          int64_t ts;
>>          av_assert0(tmpq->nb_subs);
>> +
>> +        if (tmpq->current_sub_idx >= tmpq->nb_subs)
>> +            continue;
> 
> How can this issue be reproduced ?
> 
> thx
> 
> [...]

Read a VobSub subtitle till the end:
ffmpeg -i <idx input file> -c copy -f null -

- Andreas
diff mbox

Patch

diff --git a/libavformat/mpeg.c b/libavformat/mpeg.c
index 73ade71d95..474afd06b9 100644
--- a/libavformat/mpeg.c
+++ b/libavformat/mpeg.c
@@ -930,6 +930,10 @@  static int vobsub_read_packet(AVFormatContext *s, AVPacket *pkt)
         FFDemuxSubtitlesQueue *tmpq = &vobsub->q[i];
         int64_t ts;
         av_assert0(tmpq->nb_subs);
+
+        if (tmpq->current_sub_idx >= tmpq->nb_subs)
+            continue;
+
         ts = tmpq->subs[tmpq->current_sub_idx].pts;
         if (ts < min_ts) {
             min_ts = ts;