| Message ID | 20200716072728.25072-1-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Commit | 081e3001edb67dcd55fe0f68505df1fce667476d |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avcodec/tdsc: Fix tile checks | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
Michael Niedermayer (12020-07-16): > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libswscale/tests/swscale.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libswscale/tests/swscale.c b/libswscale/tests/swscale.c > index 845ced61bb..9c0b5a4b11 100644 > --- a/libswscale/tests/swscale.c > +++ b/libswscale/tests/swscale.c > @@ -248,7 +248,7 @@ end: > if (dstStride[i]) > av_free(dst[i]); > > - return res; > + return res & 127; > } > > static void selfTest(const uint8_t * const ref[4], int refStride[4], This looks fragile: there is no local guarantee that res is not a multiple of 128, and changes in other parts of the code could have that effect. Better change the ultimate return value of main to !!res IMHO. Regards,
On Thu, Jul 16, 2020 at 09:27:27AM +0200, Michael Niedermayer wrote: > Fixes: out of array access > Fixes: crash.asf > > Found-by: anton listov <greyfarn7@yandex.ru> > Reviewed-by: anton listov <greyfarn7@yandex.ru> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/tdsc.c | 21 ++++++++++----------- > 1 file changed, 10 insertions(+), 11 deletions(-) will apply [...]
diff --git a/libavcodec/tdsc.c b/libavcodec/tdsc.c index eaea41c1f5..3617911071 100644 --- a/libavcodec/tdsc.c +++ b/libavcodec/tdsc.c @@ -390,7 +390,7 @@ static int tdsc_decode_tiles(AVCodecContext *avctx, int number_tiles) for (i = 0; i < number_tiles; i++) { int tile_size; int tile_mode; - int x, y, w, h; + int x, y, x2, y2, w, h; int ret; if (bytestream2_get_bytes_left(&ctx->gbc) < 4 || @@ -408,20 +408,19 @@ static int tdsc_decode_tiles(AVCodecContext *avctx, int number_tiles) bytestream2_skip(&ctx->gbc, 4); // unknown x = bytestream2_get_le32(&ctx->gbc); y = bytestream2_get_le32(&ctx->gbc); - w = bytestream2_get_le32(&ctx->gbc) - x; - h = bytestream2_get_le32(&ctx->gbc) - y; + x2 = bytestream2_get_le32(&ctx->gbc); + y2 = bytestream2_get_le32(&ctx->gbc); - if (x >= ctx->width || y >= ctx->height) { + if (x < 0 || y < 0 || x2 <= x || y2 <= y || + x2 > ctx->width || y2 > ctx->height + ) { av_log(avctx, AV_LOG_ERROR, - "Invalid tile position (%d.%d outside %dx%d).\n", - x, y, ctx->width, ctx->height); - return AVERROR_INVALIDDATA; - } - if (x + w > ctx->width || y + h > ctx->height) { - av_log(avctx, AV_LOG_ERROR, - "Invalid tile size %dx%d\n", w, h); + "Invalid tile position (%d.%d %d.%d outside %dx%d).\n", + x, y, x2, y2, ctx->width, ctx->height); return AVERROR_INVALIDDATA; } + w = x2 - x; + h = y2 - y; ret = av_reallocp(&ctx->tilebuffer, tile_size); if (!ctx->tilebuffer)