Browse code

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>
(cherry picked from commit 816577716bc6170bccfea3b9e865618b69a4b426)

Signed-off-by: Michael Niedermayer <michaelni@gmx.at>

wm4 authored on 2015/01/09 01:19:17
Showing 1 changed files
... ...
@@ -39,7 +39,7 @@ typedef struct DVDSubContext
39 39
   int      has_palette;
40 40
   uint8_t  colormap[4];
41 41
   uint8_t  alpha[256];
42
-  uint8_t *buf;
42
+  uint8_t  buf[0x10000];
43 43
   int      buf_size;
44 44
   int      forced_subs_only;
45 45
 #ifdef DEBUG
... ...
@@ -509,15 +509,11 @@ static int append_to_cached_buf(AVCodecContext *avctx,
509 509
 {
510 510
     DVDSubContext *ctx = avctx->priv_data;
511 511
 
512
-    if (ctx->buf_size > 0xffff - buf_size) {
512
+    if (ctx->buf_size >= sizeof(ctx->buf) - buf_size) {
513 513
         av_log(avctx, AV_LOG_WARNING, "Attempt to reconstruct "
514 514
                "too large SPU packets aborted.\n");
515
-        av_freep(&ctx->buf);
516 515
         return AVERROR_INVALIDDATA;
517 516
     }
518
-    ctx->buf = av_realloc(ctx->buf, ctx->buf_size + buf_size);
519
-    if (!ctx->buf)
520
-        return AVERROR(ENOMEM);
521 517
     memcpy(ctx->buf + ctx->buf_size, buf, buf_size);
522 518
     ctx->buf_size += buf_size;
523 519
     return 0;
... ...
@@ -533,7 +529,7 @@ static int dvdsub_decode(AVCodecContext *avctx,
533 533
     AVSubtitle *sub = data;
534 534
     int is_menu;
535 535
 
536
-    if (ctx->buf) {
536
+    if (ctx->buf_size) {
537 537
         int ret = append_to_cached_buf(avctx, buf, buf_size);
538 538
         if (ret < 0) {
539 539
             *data_size = 0;
... ...
@@ -575,7 +571,6 @@ static int dvdsub_decode(AVCodecContext *avctx,
575 575
     }
576 576
 #endif
577 577
 
578
-    av_freep(&ctx->buf);
579 578
     ctx->buf_size = 0;
580 579
     *data_size = 1;
581 580
     return buf_size;
... ...
@@ -719,7 +714,6 @@ static av_cold int dvdsub_init(AVCodecContext *avctx)
719 719
 static av_cold int dvdsub_close(AVCodecContext *avctx)
720 720
 {
721 721
     DVDSubContext *ctx = avctx->priv_data;
722
-    av_freep(&ctx->buf);
723 722
     ctx->buf_size = 0;
724 723
     return 0;
725 724
 }