[FFmpeg-devel] [PATCH 6/7] lavf/flacenc: avoid buffer overread with unexpected extradata sizes

[James Almer]

On 8/1/2017 3:33 AM, Rodger Combs wrote:
> ---
>  libavformat/flacenc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/flacenc.c b/libavformat/flacenc.c
> index 9768b6a..1906aee 100644
> --- a/libavformat/flacenc.c
> +++ b/libavformat/flacenc.c
> @@ -322,7 +322,7 @@ static int flac_write_trailer(struct AVFormatContext *s)
>      if (!c->write_header || !streaminfo)
>          return 0;
>  
> -    if (pb->seekable & AVIO_SEEKABLE_NORMAL) {
> +    if (pb->seekable & AVIO_SEEKABLE_NORMAL && (c->streaminfo || s->streams[0]->codecpar->extradata_size == FLAC_STREAMINFO_SIZE)) {

In what situation would stream extradata or c->streaminfo be smaller
than FLAC_STREAMINFO_SIZE? Nothing changes par->extradata anywhere, and
c->streaminfo is always allocated with FLAC_STREAMINFO_SIZE bytes of memory.
I have the feeling i already asked this before, but not sure if you
answered it. Apologies if you did.

You can also simplify this by just rewriting the STREAMINFO header only
if c->streaminfo is allocated, meaning updated extradata showed up in a
packet. Otherwise, you'd be rewriting the same STREAMINFO header you
already wrote at the beginning.
You could also even use ff_flac_write_header(), which already does a
buffer size check.

diff --git a/libavformat/flacenc.c b/libavformat/flacenc.c
index b894f9ef61..7392cf13c1 100644
--- a/libavformat/flacenc.c
+++ b/libavformat/flacenc.c
@@ -141,17 +141,15 @@ static int flac_write_trailer(struct
AVFormatContext *s)
     AVIOContext *pb = s->pb;
     int64_t file_size;
     FlacMuxerContext *c = s->priv_data;
-    uint8_t *streaminfo = c->streaminfo ? c->streaminfo :
-
s->streams[0]->codecpar->extradata;

-    if (!c->write_header || !streaminfo)
+    if (!c->write_header || !c->streaminfo)
         return 0;

     if (pb->seekable & AVIO_SEEKABLE_NORMAL) {
         /* rewrite the STREAMINFO header block data */
         file_size = avio_tell(pb);
-        avio_seek(pb, 8, SEEK_SET);
-        avio_write(pb, streaminfo, FLAC_STREAMINFO_SIZE);
+        avio_seek(pb, 0, SEEK_SET);
+        ff_flac_write_header(pb, c->streaminfo, FLAC_STREAMINFO_SIZE, 0);
         avio_seek(pb, file_size, SEEK_SET);
         avio_flush(pb);
     } else {