| Message ID | 20200220185056.10900-1-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avcodec/cdtoons: Correct several end of data checks in cdtoons_render_sprite() | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
LGTM On 2/20/20, Michael Niedermayer <michael@niedermayer.cc> wrote: > No testcases, found by code review when debuging issue found by oss-fuzz > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/cdtoons.c | 24 ++++++++++++++---------- > 1 file changed, 14 insertions(+), 10 deletions(-) > > diff --git a/libavcodec/cdtoons.c b/libavcodec/cdtoons.c > index 24a328352c..dc4fa6bf0b 100644 > --- a/libavcodec/cdtoons.c > +++ b/libavcodec/cdtoons.c > @@ -82,9 +82,11 @@ static int cdtoons_render_sprite(AVCodecContext *avctx, > const uint8_t *data, > for (int y = 0; y < height; y++) { > /* one scanline at a time, size is provided */ > data = next_line; > - if (data > end - 2) > + if (end - data < 2) > return 1; > line_size = bytestream_get_be16(&data); > + if (end - data < line_size) > + return 1; > next_line = data + line_size; > if (dst_y + y < 0) > continue; > @@ -94,7 +96,7 @@ static int cdtoons_render_sprite(AVCodecContext *avctx, > const uint8_t *data, > to_skip = skip; > x = 0; > while (x < width - skip) { > - int raw, size; > + int raw, size, step; > uint8_t val; > > if (data >= end) > @@ -108,20 +110,22 @@ static int cdtoons_render_sprite(AVCodecContext > *avctx, const uint8_t *data, > if (to_skip >= size) { > to_skip -= size; > if (raw) { > - data += size; > + step = size; > } else { > - data += 1; > + step = 1; > } > - if (data > next_line) > + if (next_line - data < step) > return 1; > + data += step; > continue; > } else if (to_skip) { > size -= to_skip; > - if (raw) > + if (raw) { > + if (next_line - data < to_skip) > + return 1; > data += to_skip; > + } > to_skip = 0; > - if (data > next_line) > - return 1; > } > > if (x + size >= width - skip) > @@ -129,10 +133,10 @@ static int cdtoons_render_sprite(AVCodecContext > *avctx, const uint8_t *data, > > /* either raw data, or a run of a single color */ > if (raw) { > + if (next_line - data < size) > + return 1; > memcpy(dest + x, data, size); > data += size; > - if (data > next_line) > - return 1; > } else { > uint8_t color = bytestream_get_byte(&data); > /* ignore transparent runs */ > -- > 2.17.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".
On Thu, Feb 20, 2020 at 11:32:08PM +0100, Paul B Mahol wrote:
> LGTM
will apply
thx
[...]
diff --git a/libavcodec/cdtoons.c b/libavcodec/cdtoons.c index 24a328352c..dc4fa6bf0b 100644 --- a/libavcodec/cdtoons.c +++ b/libavcodec/cdtoons.c @@ -82,9 +82,11 @@ static int cdtoons_render_sprite(AVCodecContext *avctx, const uint8_t *data, for (int y = 0; y < height; y++) { /* one scanline at a time, size is provided */ data = next_line; - if (data > end - 2) + if (end - data < 2) return 1; line_size = bytestream_get_be16(&data); + if (end - data < line_size) + return 1; next_line = data + line_size; if (dst_y + y < 0) continue; @@ -94,7 +96,7 @@ static int cdtoons_render_sprite(AVCodecContext *avctx, const uint8_t *data, to_skip = skip; x = 0; while (x < width - skip) { - int raw, size; + int raw, size, step; uint8_t val; if (data >= end) @@ -108,20 +110,22 @@ static int cdtoons_render_sprite(AVCodecContext *avctx, const uint8_t *data, if (to_skip >= size) { to_skip -= size; if (raw) { - data += size; + step = size; } else { - data += 1; + step = 1; } - if (data > next_line) + if (next_line - data < step) return 1; + data += step; continue; } else if (to_skip) { size -= to_skip; - if (raw) + if (raw) { + if (next_line - data < to_skip) + return 1; data += to_skip; + } to_skip = 0; - if (data > next_line) - return 1; } if (x + size >= width - skip) @@ -129,10 +133,10 @@ static int cdtoons_render_sprite(AVCodecContext *avctx, const uint8_t *data, /* either raw data, or a run of a single color */ if (raw) { + if (next_line - data < size) + return 1; memcpy(dest + x, data, size); data += size; - if (data > next_line) - return 1; } else { uint8_t color = bytestream_get_byte(&data); /* ignore transparent runs */
No testcases, found by code review when debuging issue found by oss-fuzz Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/cdtoons.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)