[FFmpeg-devel] [PATCH] libavformat/utils: Fix segfault on m4a cover artwork parsing

[Lazaros Koromilas]

Hello list,

I came across this bug when my MPD choked on some iTunes files while updating
its database.  Turns out that I had a few m4a files with zero-length cover
artwork tags that triggered this.  I've uploaded to the ftp server a sample
created from scratch with ffmpeg and TagLib under the name
segfault_avformat_cover_art.{m4a,txt}.  The diff at the end avoids the crash,
but I don't know if the return code is appropriate.  Maybe you want to simply
ignore those cases?  Including valgring output also.  I'm not on the list,
so please reply in person for anything else.

Thanks!


==25261== Memcheck, a memory error detector
==25261== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==25261== Using Valgrind-3.9.0 and LibVEX; rerun with -h for copyright info
==25261== Command: ./ffmpeg_g -i segfault_avformat_cover_art.m4a
==25261== 
ffmpeg started on 2014-05-27 at 13:22:02
Report written to "ffmpeg-20140527-132202.log"
ffmpeg version 2.2.git Copyright (c) 2000-2014 the FFmpeg developers
  built on May 27 2014 12:58:33 with gcc 4.9.0 (GCC) 20140507 (prerelease)
  configuration: --disable-doc --disable-everything --enable-protocol=file
  libavutil      52. 87.100 / 52. 87.100
  libavcodec     55. 65.100 / 55. 65.100
  libavformat    55. 41.100 / 55. 41.100
  libavdevice    55. 13.101 / 55. 13.101
  libavfilter     4.  5.100 /  4.  5.100
  libswscale      2.  6.100 /  2.  6.100
  libswresample   0. 19.100 /  0. 19.100
[mov,mp4,m4a,3gp,3g2,mj2 @ 0x68abde0] stream 0, timescale not set
==25261== Invalid read of size 8
==25261==    at 0x506CA3: av_buffer_ref (buffer.c:98)
==25261==    by 0x48467E: avformat_queue_attached_pictures (utils.c:520)
==25261==    by 0x48E04F: avformat_open_input (utils.c:601)
==25261==    by 0x41D403: ffmpeg_parse_options (ffmpeg_opt.c:850)
==25261==    by 0x40F706: main (ffmpeg.c:3736)
==25261==  Address 0x8 is not stack'd, malloc'd or (recently) free'd
==25261== 
==25261== 
==25261== Process terminating with default action of signal 11 (SIGSEGV)
==25261==  Access not within mapped region at address 0x8
==25261==    at 0x506CA3: av_buffer_ref (buffer.c:98)
==25261==    by 0x48467E: avformat_queue_attached_pictures (utils.c:520)
==25261==    by 0x48E04F: avformat_open_input (utils.c:601)
==25261==    by 0x41D403: ffmpeg_parse_options (ffmpeg_opt.c:850)
==25261==    by 0x40F706: main (ffmpeg.c:3736)
==25261==  If you believe this happened as a result of a stack
==25261==  overflow in your program's main thread (unlikely but
==25261==  possible), you can try to increase the size of the
==25261==  main thread stack using the --main-stacksize= flag.
==25261==  The main thread stack size used in this run was 8388608.
==25261== 
==25261== HEAP SUMMARY:
==25261==     in use at exit: 137,008 bytes in 78 blocks
==25261==   total heap usage: 129 allocs, 51 frees, 188,107 bytes allocated
==25261== 
==25261== LEAK SUMMARY:
==25261==    definitely lost: 24 bytes in 1 blocks
==25261==    indirectly lost: 0 bytes in 0 blocks
==25261==      possibly lost: 0 bytes in 0 blocks
==25261==    still reachable: 136,984 bytes in 77 blocks
==25261==         suppressed: 0 bytes in 0 blocks
==25261== Rerun with --leak-check=full to see details of leaked memory
==25261== 
==25261== For counts of detected and suppressed errors, rerun with: -v
==25261== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 1 from 1)
zsh: segmentation fault (core dumped)  valgrind ./ffmpeg_g -i segfault_avformat_cover_art.m4a


--- libavformat/utils.c.orig    2014-05-27 13:39:42.905773093 +0300
+++ libavformat/utils.c 2014-05-27 13:50:12.369892155 +0300
@@ -517,6 +517,8 @@
         if (s->streams[i]->disposition & AV_DISPOSITION_ATTACHED_PIC &&
             s->streams[i]->discard < AVDISCARD_ALL) {
             AVPacket copy = s->streams[i]->attached_pic;
+            if (copy.size <= 0)
+                return AVERROR(EINVAL);
             copy.buf = av_buffer_ref(copy.buf);
             if (!copy.buf)
                 return AVERROR(ENOMEM);