| Message ID | 20190919221706.16529-3-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Commit | 093b6894bfc2a8f0804f1c774bfe7827e44ffdb2 |
| Headers | show |
applied On 9/20/19, Andreas Rheinhardt <andreas.rheinhardt@gmail.com> wrote: > The MPEG-1/2 decoder uses avpriv_find_start_code to search for start > codes and worked with the resulting start code before checking that it > is really a start code of a slice. In particular, if the picture is so > big that a slice_vertical_position_extension is present, it added the > slice_vertical_position_extension as if it had a slice. Then a left > shift is performed, without making sure that the value to be shifted is > nonnegative. > Afterwards the end result is checked, but even if a start code of a > non-slice has been found, it might pass these checks: If > slice_vertical_position_extension is present a start code < > SLICE_MIN_START_CODE can lead to a macroblock-row index that appears > valid. Furthermore, the left shift might make an invalid start code > appear valid by discarding the highest bit. > This has been fixed by checking directly after avpriv_find_start_code > has returned. > > Fixes ticket #8162 (which is about the undefined left shifts). > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > libavcodec/mpeg12dec.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c > index 83e537884b..1904b75213 100644 > --- a/libavcodec/mpeg12dec.c > +++ b/libavcodec/mpeg12dec.c > @@ -2011,13 +2011,15 @@ static int slice_decode_thread(AVCodecContext *c, > void *arg) > > start_code = -1; > buf = avpriv_find_start_code(buf, s->gb.buffer_end, > &start_code); > + if (start_code < SLICE_MIN_START_CODE || start_code > > SLICE_MAX_START_CODE) > + return AVERROR_INVALIDDATA; > mb_y = start_code - SLICE_MIN_START_CODE; > if (s->codec_id != AV_CODEC_ID_MPEG1VIDEO && s->mb_height > > 2800/16) > mb_y += (*buf&0xE0)<<2; > mb_y <<= field_pic; > if (s->picture_structure == PICT_BOTTOM_FIELD) > mb_y++; > - if (mb_y < 0 || mb_y >= s->end_mb_y) > + if (mb_y >= s->end_mb_y) > return AVERROR_INVALIDDATA; > } > } > -- > 2.20.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c index 83e537884b..1904b75213 100644 --- a/libavcodec/mpeg12dec.c +++ b/libavcodec/mpeg12dec.c @@ -2011,13 +2011,15 @@ static int slice_decode_thread(AVCodecContext *c, void *arg) start_code = -1; buf = avpriv_find_start_code(buf, s->gb.buffer_end, &start_code); + if (start_code < SLICE_MIN_START_CODE || start_code > SLICE_MAX_START_CODE) + return AVERROR_INVALIDDATA; mb_y = start_code - SLICE_MIN_START_CODE; if (s->codec_id != AV_CODEC_ID_MPEG1VIDEO && s->mb_height > 2800/16) mb_y += (*buf&0xE0)<<2; mb_y <<= field_pic; if (s->picture_structure == PICT_BOTTOM_FIELD) mb_y++; - if (mb_y < 0 || mb_y >= s->end_mb_y) + if (mb_y >= s->end_mb_y) return AVERROR_INVALIDDATA; } }
The MPEG-1/2 decoder uses avpriv_find_start_code to search for start codes and worked with the resulting start code before checking that it is really a start code of a slice. In particular, if the picture is so big that a slice_vertical_position_extension is present, it added the slice_vertical_position_extension as if it had a slice. Then a left shift is performed, without making sure that the value to be shifted is nonnegative. Afterwards the end result is checked, but even if a start code of a non-slice has been found, it might pass these checks: If slice_vertical_position_extension is present a start code < SLICE_MIN_START_CODE can lead to a macroblock-row index that appears valid. Furthermore, the left shift might make an invalid start code appear valid by discarding the highest bit. This has been fixed by checking directly after avpriv_find_start_code has returned. Fixes ticket #8162 (which is about the undefined left shifts). Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavcodec/mpeg12dec.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)