Browse Source

avcodec/dvdsubdec: fix accessing dangling pointers

dvdsub_decode() can call append_to_cached_buf() 2 times, the second time
with ctx->buf as argument. If the second append_to_cached_buf() reallocs
ctx->buf, the argument will be a pointer to the previous, freed block.
This can cause invalid reads at least with some fuzzed files - and
possibly with valid files.

Since packets can apparently not be larger than 64K (even if packets are
combined), just use a fixed size buffer. It will be allocated as part of
the DVDSubContext, and although some memory is "wasted", it's relatively
minimal by modern standards and should be acceptable.

Signed-off-by: Michael Niedermayer <michaelni@gmx.at>
tags/n2.6
wm4 Michael Niedermayer 10 years ago
parent
commit
816577716b
1 changed files with 3 additions and 9 deletions
  1. +3
    -9
      libavcodec/dvdsubdec.c

+ 3
- 9
libavcodec/dvdsubdec.c View File

@@ -39,7 +39,7 @@ typedef struct DVDSubContext
int has_palette; int has_palette;
uint8_t colormap[4]; uint8_t colormap[4];
uint8_t alpha[256]; uint8_t alpha[256];
uint8_t *buf;
uint8_t buf[0x10000];
int buf_size; int buf_size;
int forced_subs_only; int forced_subs_only;
#ifdef DEBUG #ifdef DEBUG
@@ -509,15 +509,11 @@ static int append_to_cached_buf(AVCodecContext *avctx,
{ {
DVDSubContext *ctx = avctx->priv_data; DVDSubContext *ctx = avctx->priv_data;


if (ctx->buf_size > 0xffff - buf_size) {
if (ctx->buf_size >= sizeof(ctx->buf) - buf_size) {
av_log(avctx, AV_LOG_WARNING, "Attempt to reconstruct " av_log(avctx, AV_LOG_WARNING, "Attempt to reconstruct "
"too large SPU packets aborted.\n"); "too large SPU packets aborted.\n");
av_freep(&ctx->buf);
return AVERROR_INVALIDDATA; return AVERROR_INVALIDDATA;
} }
ctx->buf = av_realloc(ctx->buf, ctx->buf_size + buf_size);
if (!ctx->buf)
return AVERROR(ENOMEM);
memcpy(ctx->buf + ctx->buf_size, buf, buf_size); memcpy(ctx->buf + ctx->buf_size, buf, buf_size);
ctx->buf_size += buf_size; ctx->buf_size += buf_size;
return 0; return 0;
@@ -533,7 +529,7 @@ static int dvdsub_decode(AVCodecContext *avctx,
AVSubtitle *sub = data; AVSubtitle *sub = data;
int is_menu; int is_menu;


if (ctx->buf) {
if (ctx->buf_size) {
int ret = append_to_cached_buf(avctx, buf, buf_size); int ret = append_to_cached_buf(avctx, buf, buf_size);
if (ret < 0) { if (ret < 0) {
*data_size = 0; *data_size = 0;
@@ -575,7 +571,6 @@ static int dvdsub_decode(AVCodecContext *avctx,
} }
#endif #endif


av_freep(&ctx->buf);
ctx->buf_size = 0; ctx->buf_size = 0;
*data_size = 1; *data_size = 1;
return buf_size; return buf_size;
@@ -719,7 +714,6 @@ static av_cold int dvdsub_init(AVCodecContext *avctx)
static av_cold int dvdsub_close(AVCodecContext *avctx) static av_cold int dvdsub_close(AVCodecContext *avctx)
{ {
DVDSubContext *ctx = avctx->priv_data; DVDSubContext *ctx = avctx->priv_data;
av_freep(&ctx->buf);
ctx->buf_size = 0; ctx->buf_size = 0;
return 0; return 0;
} }


Loading…
Cancel
Save