[FFmpeg-trac] #554(avcodec:new): Buffer overflow in dvbsubdec.c

[FFmpeg]

#554: Buffer overflow in dvbsubdec.c
------------------------------------+-----------------------------------
             Reporter:  mihnea      |                    Owner:
                 Type:  defect      |                   Status:  new
             Priority:  normal      |                Component:  avcodec
              Version:  git-master  |               Resolution:
             Keywords:  dvbsub      |               Blocked By:
             Blocking:              |  Reproduced by developer:  0
Analyzed by developer:  0           |
------------------------------------+-----------------------------------
Description changed by cehoyos:

Old description:

> Hi.
>
> The function dvbsub_parse_pixel_data_block() in libavcodec/dvbsubdec.c is
> prone to overflowing the region->pbuf buffer. That buffer is
> region->width*region->height bytes in length, but the check for overflow
> is done like this:
>
> if (x_pos > region->width || y_pos > region->height)
>
> The comparisons should obviously use greater than equal instead of
> greater, since you never want to write at region->height * region->width
> + something. However, if I change them, the "invalid object location"
> message triggers all the time because y_pos is incremented a few lines
> above like this:
>
> if ((y_pos & 1) != top_bottom)
>     y_pos++;
>
> I suppose this is trying to align the starting line to odd or even to
> account for interlacing. I'm not sure how this works for progressive
> streams since I don't know anything about how DVB subtitles are encoded,
> but with a progressive stream it always reaches this piece of code with
> y_pos = region->height - 1, so the increment makes y_pos invalid, causing
> a buffer overflow with the current code, or triggering the error message
> if the comparison is fixed.

New description:

 Hi.

 The function dvbsub_parse_pixel_data_block() in libavcodec/dvbsubdec.c is
 prone to overflowing the region->pbuf buffer. That buffer is
 region->width*region->height bytes in length, but the check for overflow
 is done like this:
 {{{
 if (x_pos > region->width || y_pos > region->height)
 }}}
 The comparisons should obviously use greater than equal instead of
 greater, since you never want to write at region->height * region->width +
 something. However, if I change them, the "invalid object location"
 message triggers all the time because y_pos is incremented a few lines
 above like this:
 {{{
 if ((y_pos & 1) != top_bottom)
     y_pos++;
 }}}
 I suppose this is trying to align the starting line to odd or even to
 account for interlacing. I'm not sure how this works for progressive
 streams since I don't know anything about how DVB subtitles are encoded,
 but with a progressive stream it always reaches this piece of code with
 y_pos = region->height - 1, so the increment makes y_pos invalid, causing
 a buffer overflow with the current code, or triggering the error message
 if the comparison is fixed.

--

-- 
Ticket URL: <https://ffmpeg.org/trac/ffmpeg/ticket/554#comment:3>
FFmpeg <http://ffmpeg.org>
FFmpeg issue tracker