On Wed, Jan 07, 2015 at 11:57:51PM +0100, wm4 wrote: > 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.
this breaks fate-sub2video
> ---
> Not sure if GetBitContext needs stricter padding and alignment for this
> buffer?
> ---
> libavcodec/dvdsubdec.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/libavcodec/dvdsubdec.c b/libavcodec/dvdsubdec.c
> index 1cc34ea..9e5ca0a 100644
> --- a/libavcodec/dvdsubdec.c
> +++ b/libavcodec/dvdsubdec.c
> @@ -39,7 +39,7 @@ typedef struct DVDSubContext
> int has_palette;
> uint8_t colormap[4];
> uint8_t alpha[256];
> - uint8_t *buf;
> + uint8_t buf[0x10000];
theres a if(ctx->buf) which would be a if(1) with this change
> int buf_size;
> int forced_subs_only;
> #ifdef DEBUG
> @@ -512,12 +512,8 @@ static int append_to_cached_buf(AVCodecContext *avctx,
> if (ctx->buf_size > 0xffff - buf_size) {
ths 0xffff could be changed to sizeof()
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
