[FFmpeg-trac] #554(avcodec:new): Buffer overflow in dvbsubdec.c
FFmpeg
trac at avcodec.org
Fri Oct 14 13:25:21 CEST 2011
#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
More information about the FFmpeg-trac
mailing list