shithub: aacdec

Info  •  Files  •  Log

mp4read: moovin should not alter caller's g_atom

moov blocks contain MP4 file metadata. mp4read.c parses moov blocks
using moovin, which recursively calls parse to process subblocks.  In
order to use the parse function, moovin modifies the value of g_atom,
which is a global, static variable. When moovin returns, the position
of the g_atom pointer has thus changed. For example it might be
pointing to the last element of a trak[], meaning that g_atom++
pushes it out-of-bounds.

the result is a buffer-over-read later at line 767.

fix: moovin should not modify g_atom from the point of view of its
caller. Save g_atom's value at the beginning of moovin calls and reset
it before returning.

fixes #34.

--- a/frontend/mp4read.c

+++ b/frontend/mp4read.c

@@ -797,7 +797,8 @@

 {

     long apos = ftell(g_fin);

     uint32_t atomsize;

-    int err;

+    creator_t *old_atom = g_atom;

+    int err, ret = sizemax;

     static creator_t mvhd[] = {

         {ATOM_NAME, "mvhd"},

@@ -841,8 +842,11 @@

     g_atom = mvhd;

     atomsize = sizemax + apos - ftell(g_fin);

-    if (parse(&atomsize) < 0)

+    if (parse(&atomsize) < 0) {

+        g_atom = old_atom;

         return ERR_FAIL;

+    }

+

     fseek(g_fin, apos, SEEK_SET);

     while (1)

@@ -856,13 +860,16 @@

         err = parse(&atomsize);

         //fprintf(stderr, "SIZE: %x/%x\n", atomsize, sizemax);

         if (err >= 0)

-            return sizemax;

-        if (err != ERR_UNSUPPORTED)

-            return err;

+            break;

+        if (err != ERR_UNSUPPORTED) {

+            ret = err;

+            break;

+        }

         //fprintf(stderr, "UNSUPP\n");

     }

-    return sizemax;

+    g_atom = old_atom;

+    return ret;

 }